jsonapi-authorization icon indicating copy to clipboard operation
jsonapi-authorization copied to clipboard

New feature: Support resource caching

Open thibaudgg opened this issue 7 years ago • 10 comments

Hi,

While testing the 0.9 JR caching feature I encountered the following error:

Internal Server Error: undefined method `_model' for #<JSONAPI::CachedResourceFragment:0x007fee59c5d6a0> /Users/Thibaud/.gem/ruby/2.4.1/gems/jsonapi-authorization-1.0.0.alpha2/lib/jsonapi/authorization/authorizing_processor.rb:42:in `block in authorize_include_directive'
/Users/Thibaud/.gem/ruby/2.4.1/gems/jsonapi-authorization-1.0.0.alpha2/lib/jsonapi/authorization/authorizing_processor.rb:41:in `each'
/Users/Thibaud/.gem/ruby/2.4.1/gems/jsonapi-authorization-1.0.0.alpha2/lib/jsonapi/authorization/authorizing_processor.rb:41:in `authorize_include_directive'
/Users/Thibaud/.gem/ruby/2.4.1/gems/activesupport-5.0.2/lib/active_support/callbacks.rb:382:in `block in make_lambda'
/Users/Thibaud/.gem/ruby/2.4.1/gems/activesupport-5.0.2/lib/active_support/callbacks.rb:242:in `block in simple'
/Users/Thibaud/.gem/ruby/2.4.1/gems/activesupport-5.0.2/lib/active_support/callbacks.rb:456:in `block in call'
/Users/Thibaud/.gem/ruby/2.4.1/gems/activesupport-5.0.2/lib/active_support/callbacks.rb:456:in `each'
/Users/Thibaud/.gem/ruby/2.4.1/gems/activesupport-5.0.2/lib/active_support/callbacks.rb:456:in `call'
/Users/Thibaud/.gem/ruby/2.4.1/gems/activesupport-5.0.2/lib/active_support/callbacks.rb:101:in `__run_callbacks__'
/Users/Thibaud/.gem/ruby/2.4.1/gems/activesupport-5.0.2/lib/active_support/callbacks.rb:750:in `_run_find_callbacks'
/Users/Thibaud/.gem/ruby/2.4.1/gems/activesupport-5.0.2/lib/active_support/callbacks.rb:90:in `run_callbacks'
/Users/Thibaud/.gem/ruby/2.4.1/gems/jsonapi-resources-0.9.0/lib/jsonapi/processor.rb:57:in `block in process'
/Users/Thibaud/.gem/ruby/2.4.1/gems/activesupport-5.0.2/lib/active_support/callbacks.rb:97:in `__run_callbacks__'
/Users/Thibaud/.gem/ruby/2.4.1/gems/activesupport-5.0.2/lib/active_support/callbacks.rb:750:in `_run_operation_callbacks'
/Users/Thibaud/.gem/ruby/2.4.1/gems/activesupport-5.0.2/lib/active_support/callbacks.rb:90:in `run_callbacks'
/Users/Thibaud/.gem/ruby/2.4.1/gems/jsonapi-resources-0.9.0/lib/jsonapi/processor.rb:56:in `process'
/Users/Thibaud/.gem/ruby/2.4.1/gems/jsonapi-resources-0.9.0/lib/jsonapi/operation.rb:16:in `process'
/Users/Thibaud/.gem/ruby/2.4.1/gems/jsonapi-resources-0.9.0/lib/jsonapi/operation_dispatcher.rb:58:in `block in process_operation'
/Users/Thibaud/.gem/ruby/2.4.1/gems/jsonapi-resources-0.9.0/lib/jsonapi/operation_dispatcher.rb:63:in `with_default_handling'
/Users/Thibaud/.gem/ruby/2.4.1/gems/jsonapi-resources-0.9.0/lib/jsonapi/operation_dispatcher.rb:57:in `process_operation'
/Users/Thibaud/.gem/ruby/2.4.1/gems/jsonapi-resources-0.9.0/lib/jsonapi/operation_dispatcher.rb:29:in `block (2 levels) in process'
/Users/Thibaud/.gem/ruby/2.4.1/gems/jsonapi-resources-0.9.0/lib/jsonapi/operation_dispatcher.rb:28:in `each'
/Users/Thibaud/.gem/ruby/2.4.1/gems/jsonapi-resources-0.9.0/lib/jsonapi/operation_dispatcher.rb:28:in `block in process'
/Users/Thibaud/.gem/ruby/2.4.1/gems/jsonapi-resources-0.9.0/lib/jsonapi/operation_dispatcher.rb:46:in `transaction'
/Users/Thibaud/.gem/ruby/2.4.1/gems/jsonapi-resources-0.9.0/lib/jsonapi/operation_dispatcher.rb:24:in `process'
/Users/Thibaud/.gem/ruby/2.4.1/gems/jsonapi-resources-0.9.0/lib/jsonapi/acts_as_resource_controller.rb:86:in `block in process_operations'
/Users/Thibaud/.gem/ruby/2.4.1/gems/activesupport-5.0.2/lib/active_support/callbacks.rb:97:in `__run_callbacks__'
/Users/Thibaud/.gem/ruby/2.4.1/gems/activesupport-5.0.2/lib/active_support/callbacks.rb:750:in `_run_process_operations_callbacks'
/Users/Thibaud/.gem/ruby/2.4.1/gems/activesupport-5.0.2/lib/active_support/callbacks.rb:90:in `run_callbacks'
/Users/Thibaud/.gem/ruby/2.4.1/gems/jsonapi-resources-0.9.0/lib/jsonapi/acts_as_resource_controller.rb:85:in `process_operations'
/Users/Thibaud/.gem/ruby/2.4.1/gems/jsonapi-resources-0.9.0/lib/jsonapi/acts_as_resource_controller.rb:77:in `process_request'
/Users/Thibaud/.gem/ruby/2.4.1/gems/jsonapi-resources-0.9.0/lib/jsonapi/acts_as_resource_controller.rb:16:in `index'
/Users/Thibaud/.gem/ruby/2.4.1/gems/actionpack-5.0.2/lib/action_controller/metal/basic_implicit_render.rb:4:in `send_action'
/Users/Thibaud/.gem/ruby/2.4.1/gems/actionpack-5.0.2/lib/abstract_controller/base.rb:188:in `process_action'
/Users/Thibaud/.gem/ruby/2.4.1/gems/actionpack-5.0.2/lib/action_controller/metal/rendering.rb:30:in `process_action'
/Users/Thibaud/.gem/ruby/2.4.1/gems/actionpack-5.0.2/lib/abstract_controller/callbacks.rb:20:in `block in process_action'
/Users/Thibaud/.gem/ruby/2.4.1/gems/activesupport-5.0.2/lib/active_support/callbacks.rb:126:in `call'
/Users/Thibaud/.gem/ruby/2.4.1/gems/activesupport-5.0.2/lib/active_support/callbacks.rb:506:in `block (2 levels) in compile'
/Users/Thibaud/.gem/ruby/2.4.1/gems/activesupport-5.0.2/lib/active_support/callbacks.rb:455:in `call'
/Users/Thibaud/.gem/ruby/2.4.1/gems/activesupport-5.0.2/lib/active_support/callbacks.rb:448:in `block (2 levels) in around'
/Users/Thibaud/.gem/ruby/2.4.1/gems/activesupport-5.0.2/lib/active_support/callbacks.rb:286:in `block (2 levels) in halting'

JSONAPI::CachedResourceFragment object doesn't have a _model, should you skip authorization on cached objects?

thibaudgg avatar May 24 '17 14:05 thibaudgg

That's a good question. I've been wondering what would a good strategy around the caching behavior look like.

Correct me if I'm mistaken, but wouldn't the first time resources are fetched (and then cached) be ran though the authorization properly? As in, one visitor would trigger the authorizations and then subsequent visitors would just use the cache as-is, as the authorizations had already been ran once?

I'm just pondering on what kinds of authorization leaks would be possible if we didn't authorize for cached resources 🤔. A poisoned cache with e.g. admin-only visible resources have a real possibility of being visible to normal visitors that way.

These things are hopefully known to people who do enable caching, and act accordingly. I haven't really looked into how JR does caching. Do you know how JR caches things that can vary based on who is fetching the resources? Would there be a way for us to warn or error if an invalid configuration were to be used? Or provide some sort of guidance that users won't accidentally cause authorization leaks?

We should probably skip authorization on cached objects. However, it would be crucial to do that in a way that would be documented properly and if possible, we'd guide developers to design their caching strategy with authorization in mind, automatically.

valscion avatar May 26 '17 07:05 valscion

You're correct, this could depend on how the cache key is built. However, now that I more thought about it, you would better off with simplest keys possible to increase the reusability of the cached object. Giving that, I think it would be safest to actually authorize each cached resource. If the resource is cached properly the authorization process shouldn't trigger any extra query.

@valscion do you think it would be possible to update the logic to not depends on the #_model method and only uses the shared attributes by the Resource and CachedResourceFragment objects?

thibaudgg avatar May 29 '17 14:05 thibaudgg

I haven't looked into how the code over at jsonapi-resources looks like. We will need to have access to the ActiveRecord objects, though, or otherwise come up with another pattern for authorizing cached resources instead of models, and that doesn't sound so nice.

If you're able to fiddle around and create some sort of proof of concept about authorizing cached resources, we could have something concrete to talk about :relaxed:

valscion avatar May 30 '17 07:05 valscion

Ok, thanks for the feedback.

I agree that not going without the ActiveRecord objects doesn't sound that easy. We'll have to hold on that feature, I'll maybe go back to it later.

thibaudgg avatar May 30 '17 13:05 thibaudgg

Bump.

I'd love caching support for this gem. I don't mind having potentially lots of cache keys (i.e. combining the current_user#cache_key with the jsonapi-resources Resource cache key) in my instance because the cache would be short-lived.

joshkinabrew avatar Nov 07 '17 19:11 joshkinabrew

Great @joshkinabrew! Would you be willing to devote some time to come up with a proof of concept for this?

This was my comment before, and it still applies:

If you're able to fiddle around and create some sort of proof of concept about authorizing cached resources, we could have something concrete to talk about ☺️


I am open to helping get this implemented, but as we don't currently need the caching support at work, I don't have time to devote to this feature.

valscion avatar Nov 08 '17 09:11 valscion

I did some experiments with caching yesterday, but most changes that I made was in jsonapi-resources CachedResourceFragment class.

I'm exposing the original model into cache, so jsonapi-authorizer works just fine for us, there is no user specific cache key at the moment as we don't currently need the that feature, but i'm going to take a look next week.

harisadam avatar Sep 07 '18 06:09 harisadam

Thanks @harisadam! Let us know how it goes :relaxed:

valscion avatar Sep 07 '18 06:09 valscion

Hi, guys!

Do you have some news about this issue?

rvlaraujo avatar Jan 25 '23 19:01 rvlaraujo

Nope

valscion avatar Jan 26 '23 07:01 valscion