Feature support for Issue 25

[jeremyjaybaker]

Hello, I'm working on a version of the authorizing processor that can support accept a resource instead of a class for create? and update? actions as referenced in #25 and several other issues.

The current behavior (only a class is given to create? and update?) is on by default. Users may switch to a resource-oriented create? and update? by toggling a configuration option. I was thinking about something like this:

JSONAPI::Authorization.configure do |config|
  # Both are false by default
  config.access_resource_on_create = true
  config.access_resource_on_update = true
end

The configuration option changes how the callbacks are defined. That way we don't have to worry about introducing breaking changes to the default behavior, ie in authorizing_processor.rb:

...
if config.access_resource_on_create
    # I'm not entirely sure that this is the correct way to hook into the save callback,
    # but this at least shows what my intent is
    set_callback :save, :around, :authorize_create_resource_with_resource
else
    set_callback :create_resource, :before, :authorize_create_resource
end

if config.access_resource_on_update
    set_callback :save, :around, :authorize_replace_fields_with_resource
else
    set_callback :replace_fields, :before, :authorize_replace_fields
end
...

Assuming that I address edge cases that you mentioned in issue 25 (with the article.comments = comments example) and write corresponding tests and documentation, is this a feature that you would be willing to add to this gem? It's something that we need at my company so I plan on working on it regardless, but I know several other people here have also requested similar functionality.

[valscion]

Hi! Thanks for opening this issue first to discuss new additions ☺️

Could you tell me a bit more about your use case? What does your create policy methods look like if you need the resource to be there?

[jeremyjaybaker]

Sure! One of our apps is an educational dashboard that teachers can use to create and manage assignments for their students. If we consider authorization from only a class perspective, then it makes sense that anyone tagged as an educator can create, modify, and delete assignments.

For resource-specific validation, we can do most of it with model-level level constraints. If a teacher manages students 1 and 2 but tries to create an assignment for student 3, that's not an action that makes sense to perform regardless of who initiated the action. However, certain actions can look completely valid from the model level but could be performed by an unauthorized user which means model constraints aren't really an option. For example, a teacher could mark an unrelated student's assignment as completed (ie, update a single timestamp). The only way we can tell that this action is invalid is knowing who performed the action and which resource it's being performed on, hence we need more than just the model's class in the policy.

I'm open to suggestions on how to handle such a use case that doesn't involve my feature suggestion. I just haven't found any myself. I also feel like only accepting a class is somewhat misleading if you already understand vanilla Pundit since Pundit's docs show many examples that incorporate the resource directly into create and update authorization methods.

Oh, and I forgot the last part of you question. This is an example of what my policies look like:

def create?
  if record.class.try(:descends_from_active_record?)
    user == record.instructor_profile
  else
    user.is_educator? || user.is_homeschooler?
  end
end

It have it accepting both a class and a record for now. I have some custom controller code for create and update operations where I explicitly run these policies which also means that my API isn't fully JSONAPI-compliant.

[valscion]

Thanks for explaining the use case 😊. How does the

record.instructor_profile

get populated before the create? policy method is called? Is that part of your custom controller logic?

I'm mainly asking to gauge whether Authorization of operations touching relationships would work for your use case.

Can you give that doc a look? I'd be curious to hear if there's some edge case I'm missing that the relationship authorizations can't handle

[valscion]

Also, if you could provide an example API request for the create call + tell me what ActiveRecord models and associations are being touched there, I think I'd have all the information about your use case that I need right now 😊

[jeremyjaybaker]

I'll need a day or so to take a look at the docs and form a proper response. It looks like they may actually cover the uses cases I need, I just want to make sure I fully understand how JSONAPI::Authorization ties into relationship features first.

[valscion]

Thanks! Take your time 😊. I'd be super happy if you find any weirdnesses in the docs and take some time to improve them or ask questions.

This is the first version of the docs so there might be some unanswered questions, or some confusing ways of saying things. If we can improve them, everyone would benefit.

Let me also know if relationship authorization indeed solves your problems. I haven't heard much feedback about it, and would love to hear how it goes

[jeremyjaybaker]

Alright, sorry it took me a while! I'll discuss create and update separately since they end up being rather different. I'll stick to using the Assignment class as the example resource that we're trying to authorize actions on. Here's the relevant columns from the Assignment table we use and it's foreign relationships:

• to_be_assigned_at (timestamp)
• started_at (timestamp)
• completed_at (timestamp)
• student_id (foreign key to users table)
• instructor_id (foreign key to users table)
• content_id (foreign key to content table)

For authorizing creation of an assignment, it looks like the authorization of relationships does indeed work for our use case. We would need to define the methods create_with_student and create_with_instructor and to ensure that the initiating user can only create assignments for their own students.

For updating assignments, I still don't see how we could use this feature. My original example where a teacher wants to modify any arbitrary student's assignment doesn't touch any relationships on the resource. There's two custom relationship methods available for PATCH requests according to the docs: replace_student?(new_student) and remove_student?, neither of which could properly check to see if an unrelated teacher is trying to update a student's assignment.

I'm also confused about the examples given at https://github.com/venuu/jsonapi-authorization/blob/master/docs/relationship-authorization.md#custom-relationship-authorization-method since the code explicitly passes a resource instance instead of a class to the policy initialization (ArticlePolicy.new(current_user, article_1).update?). Shouldn't it be ArticlePolicy.new(current_user, Article).update?? There are several more PATCH examples that use a resource instance instead of a class.

Overall, the docs are quite thorough which I appreciate. I feel like my use case for the creation authorization should be touched on to help newcomers understand how to switch from the paradigm of accessing resources in the policy itself to relying on relationship-specific methods. This is something I can help contribute myself if you would like 😊

[valscion]

Thanks for the thorough reply. It indeed seems like the existing implementation would work for your use case.

For the PATCH/update case, JA does indeed pass the record instance, not just the record class, to the update? policy method. In there you can check the current associations of the record to see if changing an assignment is allowed.

JA can use the existing record there as the update? is called before any changes are made. If relationships would be changed in the same request, then those documented custom relationship auth methods would be used.

Does this make sense?

[jeremyjaybaker]

Hmm, I can see from the source code that you're correct, but there's something very odd happening in my request specs that is giving my policies a class instead of a resource during PATCH requests. I'll figure out what's going on and post again shortly, sorry for the misunderstanding!

[jeremyjaybaker]

I haven't had much time to look into my above issue in the past few days, but if it really does end up being a bug I'll just make a new issue.

Concerning wrapping up this issue, would you be open to me contributing some documentation to help address the misconception that users like myself have about create? being given a class instead of a resource? And if so, are there any particular guidelines you'd like me to follow?

[valscion]

Concerning wrapping up this issue, would you be open to me contributing some documentation to help address the misconception that users like myself have about create?

Sure, that would be great! I'm not sure what's the good place for that documentation, but I suppose it won't hurt even if it would be in a suboptimal place.

are there any particular guidelines you'd like me to follow?

There ain't much guidelines to follow regarding docs.

If you want to provide more reasoning for why it is so, my comments in #25 (comment) can be referred to — particularly, that JA runs authorization checks in a before hook just to make sure no unauthorized changes ever go all the way to database.

[valscion]

Hiya @jeremyjaybaker, what's the situation regarding this thread? Have you been able to resolve your issues? ☺️

[jeremyjaybaker]

Hello, sorry I completely vanished! My company just moved across the US and things have been so hectic that this slipped past me. I'll address this issue this week so we can get it closed out.

[valscion]

No problem, take your time ☺️

[valscion]

Just checking up @jeremyjaybaker, is this issue still timely for you?