1

Suppose I have a scenario where we have Users and each user can create their own Projects.

I'm trying to limit the Show action of my Rails controller to only allow admin or the owner of the project to be able to go through Show action.

The problem I am facing is, perhaps I'm misunderstanding on how to use Scopes in Pundit.

My Show action looks like this:

  def show
    project = policy_scope(Project).find_by({id: project_params[:id]})

    if project
      render json: project
    else
      render json: { error: "Not found" }, status: :not_found
    end
  end

My Pundit Scope class looks like this:

  class Scope < Scope

    def resolve
      if @user.admin?
        scope.all
      else
        # obviously, if non-matching user id, an ActiveRelation of  
        # empty array would be returned and subsequent find_by(...) 
        # would fail causing my controller's 'else' to execute
        # returning 404 instead of 403
        scope.where(user_id: @user.id)
      end
    end
  end

In my Rails test, I am trying to assert that non-project owner should receive a 403 forbidden:

test "show project should return forbidden if non admin viewing other user's project" do
  # "rex" here is not the owner of the project
  get project_path(@project.id), headers: @rex_authorization_header
  assert_response :forbidden
end

My test is failing. I am getting the error:

Failure:
ProjectsControllerTest#test_show_project_should_return_forbidden_if_non_admin_viewing_other_user's_project [/Users/zhang/App_Projects/LanceKit/Rails_Project/LanceKit/test/controllers/projects_controller_test.rb:40]:
Expected response to be a <403: forbidden>, but was a <404: Not Found>.
Expected: 403
  Actual: 404

I don't quite feel like I'm using Pundit correctly.

Should I be using Pundit's authorize project instead of using policy_scope(Project)... for the Show action?

I was expecting the scope.where(...) to detect the incorrect user id and return some error saying 'you are not authorized to view this resource' rather than returning results.

Zhang
  • 11,549
  • 7
  • 57
  • 87
  • I also found this Stackoverflow post: http://stackoverflow.com/questions/21172620/why-are-scope-oriented-actions-particularly-index-actions-treated-differen The answer by Rob seems to suggest using scope for index/show actions. – Zhang Jun 11 '16 at 12:56

2 Answers2

2

From what my test results are indicating to me, using scope for show action is wrong.

My finding is telling me Pundit scope are only used for filtering a set of data to only return those that matches a condition, it does NOT check whether the current_user is the owner of the resource. Pundit scope does NOT raise a 403 Forbidden error.

In other words, using scoping only in the show action will lead to a semantic bug that's saying this project with id 3 does not exist in the database for example instead of saying you are not authorized to view this project because it belongs to a different user.

A summary for myself:

  • use policy_scope for index action
  • use authorize for show, create, update, delete
  • use authorize AND policy_scope if you're not resource owner and trying to access some funky plural resource route like

    get "/user/1/projects" => "Project.index"

    in case you want to check if user is say a "project manager" or "collaborator" who is allowed to view your project. In this case, you would probably need to modify your scope code with an extra elsif clause.

In relation to my above question, I modified my project to use authorize inside my show action:

def show
    project = Project.find_by({id: project_params[:id]})

    authorize project

    if project
      render json: project
    else
      render json: { error: "Not found" }, status: :not_found
    end
  end

This then raises the expected 403 Forbidden error that my tests is expecting and thus my test passes.

Zhang
  • 11,549
  • 7
  • 57
  • 87
  • I agree with this, when accessing a show you are asking for a specific record. Pundit scopes make sense only for an index (or other collection method). – port5432 Aug 04 '16 at 12:49
0

Pundits docs regarding scopes state that you can indeed use them for the show action:

def index
  @posts = policy_scope(Post)
end

def show
  @post = policy_scope(Post).find(params[:id])
end

Just using authorize may not be enough if a User (manually) opens a URL with a id param of an instance, that she should not be able to view.

To avoid a RecordNotFound error, I used the recommended NilClassPolicy:

class NilClassPolicy < ApplicationPolicy
  class Scope < Scope
    def resolve
      raise Pundit::NotDefinedError, "Cannot scope NilClass"
    end
  end

  def show?
    false # Nobody can see nothing
  end
end
cseelus
  • 1,447
  • 1
  • 20
  • 31