0

I use pundit to handle my API policies, I have an item show that can be forbidden to user in some cases, and in other cases just restricted. By restricted I mean it's forbidden now, but if he pays he could access it then. So I need my API to respond with a specific code (402 Payment Required) so the client can invite the user to pay in order to unlock the show.

This is my current code, it only respond with 403 when pundit returns false.

Where would it be best to implement a condition to return 403 OR 402 in order to be DRY and clean?

class Api::V1::ItemController < Api::V1::BaseController
  def show
    @item = Item.find(params[:id])
    authorize @item
  end
end

class ItemPolicy < ApplicationPolicy
  def show?
    return true if record.public?

    # 403 will be generated, that's ok.
    return false if !record.band.members.include?(user)

    # If that condition is false I want to generate a 402 error at the end, not a 403.
    user.premium?
  end
end

class Api::V1::BaseController < ActionController::API
  include Pundit

  rescue_from Pundit::NotAuthorizedError, with: :user_not_authorized

  def user_not_authorized(_exception)
    # Here I've got the exception with :policy, :record and :query, 
    # also I can access :current_user so I could go for a condition, 
    # but that would include duplicated code from  ItemPolicy#show?.
    render json: { error: { message: "Access denied" } }, status: :forbidden
  end
end
adesurirey
  • 2,549
  • 2
  • 16
  • 36

2 Answers2

5

Unfortunately, Pundit cannot handle different error types out of the box. And it is built to always expect the policy's methods to return true or false false. Therefore, raising another custom error and rescuing from that in the controller will not work, because it would break view methods too.

I suggest a workaround to introduce different error types. Something like this might work:

# in the policy
class ItemPolicy < ApplicationPolicy
  def show?
    return true if record.public?
    return false unless record.band.members.include?(user)

    if user.premium?
      true
    else
      Current.specific_response_error_code = :payment_required
      false
    end
  end
end

# in the controller
class Api::V1::BaseController < ActionController::API
  include Pundit

  rescue_from Pundit::NotAuthorizedError, with: :user_not_authorized

  def user_not_authorized(_exception)
    case Current.specific_response_error_code
    when :payment_required
      render json: { error: { message: "Premium required" } }, status: :payment_required
    else
      render json: { error: { message: "Access denied" } }, status: :forbidden
    end
  end
end

I would not consider using the global CurrentAttributes a good practice, but they are part of Rails and in this case, using this global data store avoids overriding pundit internals.

You might want to read the API docs about CurrentAttributes.

spickermann
  • 100,941
  • 9
  • 101
  • 131
  • I like it ! I'll wait a bit to see if there is other options before acception your answer. Thank you ! – adesurirey Jun 12 '19 at 10:38
  • Well there's an issue now when I use `ItemPolicy#show?` for anything else than `authorize` as it raises an error. It means I can't use view helpers for example : `policy(@item).show?` still needs to return a boolean. – adesurirey Jun 12 '19 at 13:34
  • @adesurirey I get your point. After looking into the pundit source code I am thinking about a workaround. What version of Ruby on Rails do you use? – spickermann Jun 12 '19 at 13:59
  • I use rails 5.2.2.1 – adesurirey Jun 12 '19 at 14:02
  • @adesurirey I updated my answer to address your comment. – spickermann Jun 12 '19 at 15:04
  • Thanks @spickermann ! Didn't know about `CurrentAttributes`. Makes me think we could do the same with a readable instance variable `@specific_response_error_code`. Would you say it's better to use `CurrentAttributes`? – adesurirey Jun 12 '19 at 16:10
-1

create Response module in app/controllers/concerns/response.rb

module Response
  def json_response(object, status = :ok)
    render json: object, status: status
  end
end

create ExceptionHandler in app/controllers/concerns/exception_handler.rb

module ExceptionHandler
  extend ActiveSupport::Concern

  included do
    rescue_from Pundit::NotAuthorizedError, with: :unauthorized_request
  end

  private

  # JSON response with message; Status code 401 - Unauthorized
  def unauthorized_request(e)
    json_response({ message: e.message }, :unauthorized)
  end
end

in app/controllers/application_controller.rb

class ApplicationController < ActionController::API
   include Response
   include ExceptionHandler
end

THAT'S IT