Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New feature: Support resource caching #67

Open
thibaudgg opened this issue May 24, 2017 · 10 comments
Open

New feature: Support resource caching #67

thibaudgg opened this issue May 24, 2017 · 10 comments

Comments

@thibaudgg
Copy link
Contributor

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?

@valscion
Copy link
Member

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.

@thibaudgg
Copy link
Contributor Author

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?

@valscion
Copy link
Member

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 ☺️

@thibaudgg
Copy link
Contributor Author

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.

@valscion valscion changed the title Issue with resource caching New feature: Support resource caching May 30, 2017
@joshkinabrew
Copy link

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.

@valscion
Copy link
Member

valscion commented Nov 8, 2017

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.

@harisadam
Copy link

harisadam commented Sep 7, 2018

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.

@valscion
Copy link
Member

valscion commented Sep 7, 2018

Thanks @harisadam! Let us know how it goes ☺️

@rvlaraujo
Copy link

Hi, guys!

Do you have some news about this issue?

@valscion
Copy link
Member

Nope

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

5 participants