Skip to content
This repository has been archived by the owner on Dec 12, 2021. It is now read-only.

Issue with accessible_by and ActiveRecord SQL JOIN behavior #1002

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

jackryon
Copy link

I have a bit of roundabout logic for permitting a user to access a given image tagged "private" in my application.

The rules are:

  • any user can access any image where { is_private: false }
  • images where { is_private: true } are accessible to a user if she has been invited (via an invitation model) to any private gallery that contains the image. A Gallery has_many Invitations, and image has_and_belongs_to_many Galleries.

The 2 "can" statements from ability.rb that are relevant here are:

can :read, Image, is_private: false
can :read, Image, { is_private: true, galleries: { gallery_invitations: { invitee_id: user.id } } }

When calling accessible_by on a @user.images.accessible_by(current_ability), the call to @model_class.where(conditions).joins(joins) from cancan/lib/cancan/model_adapters/active_record_adapter.rb line 105 created SQL like this (note that there are some "type in" calls to accommodate polymorphic relationships, as well as other scopes being applies [acts_as_paranoid for one]):

"SELECT \"images\".* FROM \"images\" INNER JOIN \"image_galleries\" ON \"image_galleries\".\"image_id\" = \"images\".\"id\" INNER JOIN \"galleries\" ON \"galleries\".\"id\" = \"image_galleries\".\"gallery_id\" INNER JOIN \"invitations\" ON \"invitations\".\"invitable_id\" = \"galleries\".\"id\" AND \"invitations\".\"type\" IN ('Invitation') AND \"invitations\".\"invitable_type\" = 'Gallery' WHERE \"images\".\"user_id\" = 2 AND (upload_completed = 't') AND (\"images\".\"deleted_at\" IS NULL) AND ((\"images\".\"is_private\" = 't' AND \"invitations\".\"invitee_id\" = 3) OR (\"images\".\"is_private\" = 'f'))"

This query doesn't work, however. We need to use an outer left join to return the images for which there are no "join" models, or we only get the models from the second can rule, and not the first. I'm 99% sure that this works in its place:

@model_class.where(conditions).includes(joins)

Because this only gets called in the context of accessible_by, I don't think there would be any other unintended side effects.

@dideler
Copy link

dideler commented Apr 23, 2014

Thanks for your submission! The ryanb/cancan repository has been inactive since Sep 06, 2013.
Since only Ryan himself has commit permissions, the CanCan project is on a standstill.

CanCan has many open issues, including missing support for Rails 4. To keep CanCan alive, an active fork exists at cancancommunity/cancancan. The new gem is cancancan. More info is available at #994.

It would be really appreciated if you resubmit your pull request or issue to CanCanCan.

We hope to see you on the other side!

@xhoy
Copy link

xhoy commented Jul 1, 2014

Thanks for your submission! The ryanb/cancan repository has been inactive since Sep 06, 2013.
Since only Ryan himself has commit permissions, the CanCan project is on a standstill.

CanCan has many open issues, including missing support for Rails 4. To keep CanCan alive, an active fork exists at cancancommunity/cancancan. The new gem is cancancan. More info is available at #994.

If your pull request or issue is still applicable, it would be really appreciated if you resubmit it to CanCanCan.

We hope to see you on the other side!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants