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.

Sign up for GitHub

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

Jump to bottom

Filters values aren't using authorization scopes #6883

Closed
ngouy opened this issue Apr 1, 2021 · 9 comments · May be fixed by #7511
Closed

Filters values aren't using authorization scopes #6883

ngouy opened this issue Apr 1, 2021 · 9 comments · May be fixed by #7511

Comments

@ngouy
Copy link
Contributor

ngouy commented Apr 1, 2021

Did you find a bug?

filter values doesn't use policy scopes.

I understand there is a manual way to scope values of a filter, but as a collection of any resources indexes are by default scoped by the policy adapter (when there is one), so should be filter values, it can lead to a serious data breach.

I am even surprised to found nothing on the internet about that

Another possibility is that I am missing something?

Expected behavior

On the user index page, In the filter "company", I should only see my own company

Actual behavior

I see all companies

How to reproduce

class Company < ApplicationModel
  has_many :users
  has_many :admin_users
end

class AdminUser < ApplicationModel
  # my actual active_admin / devise user, the one that log into the portal
end

class User < ApplicaitonModel
  belongs_to :company
end

ActiveAdmin.register(User) {} # on this page, I have a company filter, I can try to filter by any company (I see all existing companies)
@deivid-rodriguez
Copy link
Member

Are you up for working on a fix @ngouy?

@ngouy
Copy link
Contributor Author

ngouy commented Apr 23, 2021

mhm 🤔

Have a lot of work ATM (including in my week end 😢 ) so I don't have any time for this
I would be glad to work on this but it may be in 1 month

@deivid-rodriguez
Copy link
Member

deivid-rodriguez commented Apr 23, 2021 •
edited

That'd be great, your help would be super appreciated if you end up finding some time!

@ngouy
Copy link
Contributor Author

ngouy commented Apr 27, 2021 •
edited

@deivid-rodriguez quick question from a kinda noob active admin open sourcer

What would be the best way to have access to active_admin_config or active_admin_namespaceor the current controller to have access to the authorization_adapter from the ActiveAdmin::Inputs::Filters::SelectInput class

Idea would be to override the "#collection" methods by using the adapter scope

Once this part is done the rest is formality

Another idea would be to prebuild values from scope when building filters and pass them down from the top of the chain to the input builder somewhere here https://github.com/activeadmin/activeadmin/blob/3dcb1834620f38f73ac5c1f550f37986d486f29c/lib/active_admin/filters/resource_extension.rb

@deivid-rodriguez
Copy link
Member

Hello!

I'm not fully sure, would have to dig in deeper. I think at this level you only have access to the underlying activerecord object you're creating the form for, so I guess you'd need to explicitly pass the correctly scoped collection to the form manually.

So your later idea would seem like the right way to go to me.

@ngouy
Copy link
Contributor Author

ngouy commented Apr 28, 2021

Thank you for your guidance @deivid-rodriguez

Will take a look at it

@ngouy ngouy mentioned this issue Apr 29, 2021
Closed
@ngouy
Copy link
Contributor Author

ngouy commented Apr 29, 2021 •
edited

@deivid-rodriguez I've sent the first shot
There are no tests yet, and var namings etc... are far from definitive, but on my current project it seems to work fine (for both default and "custom" relationship filter)

What do you think about this approach? I'm waiting for even a slight input before "committing" myself into righting the final version and tests

@deivid-rodriguez
Copy link
Member

@ngouy Yes, that the kind of approach I was thinking of, namely, passing a properly build collection to form filters using the collection: option.

@deivid-rodriguez deivid-rodriguez mentioned this issue May 24, 2022
Open
@javierjulio
Copy link
Member

There is a PR ready for this. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Force scope on default filters collection activeadmin/activeadmin
3 participants
@javierjulio @deivid-rodriguez @ngouy