search

artsy / garner

A set of Rack middleware and cache helpers that implement various caching strategies.
MIT License
344 stars 24 forks source link

Failing spec for garnered_find not respecting chained criteria #68

Open mzikherman opened 10 years ago

mzikherman commented 10 years ago

Ahoy there!

Here is a failing spec that shows garnered_find not respecting chained criteria.

I would expect @monger_who_is_a_thug.cheeses.garnered_find("cheddar") to return nothing, since cheddar is not one of this monger's cheeses.

I'm realizing in our API, we do build up scopes/chain criteria and then call garnered_find at the end on some object. This means that someone can make an API call for something they are authorized for, but pass in an object id for something else and get it. Something like partner.artworks.garnered_find(id) means that if you pass in the id of any artwork, as long as you have access to the given partner, you'll be able to get the details of a completely different private work.

Let me know if you think this is something we should fix in Garner, I'll try it out. I am thinking that the criteria used for the lookup needs to be added to the identity.

fancyremarker commented 10 years ago

Yeah, this was definitely never intended to work. :lollipop:

If you want to try to fix it, you'd be a mensch. However, I think that would be a major challenge, and (at least in the way I'm thinking about it) would greatly complicate some of the core object code.

dblock commented 10 years ago

This is a very real problem. Sounds like @fancyremarker is challenging @mzikherman to fixing this. I say you show him!

mzikherman commented 8 years ago

Damn you GitHub. I PR'ed something in Gravity that says 'fixes' and linked to this issue (since we were chaining something I had noticed). That automagically closed this, which in fact remains a real issue.

Though I guess it's better off as an actual issue and not a PR.

dblock commented 8 years ago

Thx @mzikherman, I reopened this PR though.