Make hook available to authorize resource?

[barelyknown]

I'd like to authorize resources using an approach like the convention followed by pundit. Here's what a typical controller action would look like (from the pundit documentation).

def update
  @post = Post.find(params[:id])
  authorize @post
  if @post.update(post_params)
    redirect_to @post
  else
    render :edit
  end
end

First you get the resource, then authorize it (which raises an exception if it fails), and then proceed.

For the index action, pundit provides the concept of a "scope" which gets the set of resources that the user has access to.

def index
  @posts = policy_scope(Post)
end

Anyhow, I don't see a straight forward way to hook into the controller methods since so much happens in the process_request_operations method. I think that it would be great for it to yield the resources so that they could be authorized in the middle of that method's execution.

What do you think?

[lgebhardt]

@barelyknown In my code we're using the before_operation method on a custom OperationsProcessor to add a callback:

require 'jsonapi/operations_processor'

class CustomOperationsProcessor < JSONAPI::ActiveRecordOperationsProcessor

  def before_operation(context, operation)
    operation.resource_klass.approve_operation(context, operation)
  end
end

We then derive all of our resources from an intermediate BaseResource that implements the approve_operation method.

We initially had a before_save callback implemented in almost exactly the same way #63 does, but I pulled it in favor of the before_operation on the OperationsProcessor. One reason is it's anticipated that users will override the save method and may inadvertently break the callback. Another is I thought having access to the full operation details gave more power to the callback.

Would this type of approach work for you?

[barelyknown]

I didn't even notice that option! Serves me right for not waiting to hear back from you on the issue.

The problem that I see is that I need to build the records to authorize the action. But, the records are built inside of the apply method. So, I like the before_operation approach, but it feels like we need a similar idea inserted between when we build the records and when we take the action.

As an example, here's CreateResourceOperation#apply:

def apply(context)
  resource = @resource_klass.create(context)
  # The resource isn't available until right here,
  # after the `before_operation` method is called.
  resource.replace_fields(@values)
  resource.save(context)

  return JSONAPI::OperationResult.new(:created, resource)

  rescue JSONAPI::Exceptions::Error => e
    return JSONAPI::OperationResult.new(e.errors[0].code, nil, e.errors)
  end
end

[lgebhardt]

I see. We could put the before_save and after_save callbacks in place and have them called from the apply method, like this:

def apply(context)
  resource = @resource_klass.create(context)
  resource.replace_fields(@values)

  resource.before_save(context)
  resource.save(context)
  resource.after_save(context)

  return JSONAPI::OperationResult.new(:created, resource)

  rescue JSONAPI::Exceptions::Error => e
    return JSONAPI::OperationResult.new(e.errors[0].code, nil, e.errors)
  end
end

That way you could choose the appropriate place for the call back, either on the operation or the resource. And users could override save on the resource without disabling the callbacks.

[barelyknown]

That seems right. Want me to make that adjustment?

[lgebhardt]

To be complete we should probably also have callbacks for before_remove and after_remove.

And the CreateHasManyAssociationOperation and RemoveHasManyAssociationOperation don't call save, but it feels like maybe we should have a callback for those.

I also wonder if we want to include any additional meta information, like which operation is making the save call, or even the operation itself.

Before you make any changes I'd like to get @dgeb's opinion on this.

[dgeb]

I've been talking with @lgebhardt and we are leaning towards making actions such as replace_fields more discrete on the resources themselves by having them internally call save (instead of having the operations processor call replace_fields then save).

@barelyknown what is your use case for a callback between replacing fields and saving the record? If you're looking for a general authentication callback, save is probably not a good trigger, since it is not called for every operation.

[barelyknown]

I didn't want a callback between replace_fields and save. I wanted one between getting the resource and save. The use case is authorizing that the action can be taken on the resource by the user. I am using pundit to manage authorization.

[lgebhardt]

Sorry, I misunderstood what the issue was. It makes a lot more sense now.

[lgebhardt]

I'm going to start a branch for calling the save method from the resource. That will let us move the origination of any callbacks into the resource.

@dgeb and I talked about changing the operations processor callbacks to use the Rails callback chain pattern. This should be cleaner than the way it currently is. I'll also rework the operations processor callbacks in this branch.

We still need to decide which, if any, callbacks the resource will support by default.

[dgeb]

Thanks for the clarification. I was thrown by the location of @lgebhardt's before_save hook in his code snippet above:

  resource = @resource_klass.create(context)
  resource.replace_fields(@values)

  resource.before_save(context)
  resource.save(context)
  resource.after_save(context)

I'd prefer that save be called from within the call to replace_fields. Sounds like that won't be an issue for you, as long as you have a chance to authorize the operation before it's invoked.

[dgeb]

(whoops - my reply above was sent before seeing either of @lgebhardt's replies)

[barelyknown]

Let me know if you need a hand with it.

[lgebhardt]

@barelyknown, if you want to help that would be great. Are you up for reworking the OperationsProcessor callbacks?

Ideally they would use ActiveSupport::Callbacks (http://apidock.com/rails/ActiveSupport/Callbacks). For example my custom operations processor above would become:

class CustomOperationsProcessor < JSONAPI::ActiveRecordOperationsProcessor
  set_callback :apply, :before, :approve_operation
  #or
  before_apply :approve_operation

  def approve_operation(context, operation)
    operation.resource_klass.approve_operation(context, operation)
  end
end

[barelyknown]

How are you imagining the callback would have access to the resources that had been loaded by the operation?