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

Restricting specific fields visible in show/edit #1862

Open
sedubois opened this issue Jan 9, 2021 · 12 comments
Open

Restricting specific fields visible in show/edit #1862

sedubois opened this issue Jan 9, 2021 · 12 comments
Labels
bug breakages in functionality that is implemented fields new fields, displaying and editing data fields-context

Comments

@sedubois
Copy link
Contributor

sedubois commented Jan 9, 2021

  • What were you trying to do?

The authorization/pundit system allows to restrict which resources an admin can view/edit, but how to authorize them to only view/edit specific fields within specific records? The authorization doc does not seem to cover this.

  • What did you end up with (logs, or, even better, example apps are great!)?

I did not find a way to do this.

  • What versions are you running?
    • Rails 6..1.1
    • administrate 0.14.0
@sedubois sedubois added the bug breakages in functionality that is implemented label Jan 9, 2021
@pablobm
Copy link
Collaborator

pablobm commented Jan 14, 2021

Have a look at the documentation for Pundit. A policy object typically includes the current user and the resource to be authorized. In Administrate, we have an ApplicationPolicy superclass, which stores both as @user and @record:

def initialize(user, record)
@user = user
@record = record
end

Then these can be used in the policy methods. Here's an example from the example app:

def update?
record.address_state == "AZ"
end

Does this answer your question? Perhaps we could improve the documentation on this point.

@pablobm
Copy link
Collaborator

pablobm commented Jan 14, 2021

Oh, wait. You say fields, not records! Apologies, I see now...

I don't think there's a way to do it at the moment, no 🤔

@pablobm
Copy link
Collaborator

pablobm commented Jan 21, 2021

Thinking a bit about this, perhaps a way to achieve it would be to use different dashboards for different users. Say that you have a User model, and only admins can access some information. The UserDashboard could have limited fields, whereas another ActiveRecord model, AdministrableUser (or something like that) would inherit from User and have its own dashboard, while only accessible by admins.

I haven't tried this though, and it would quickly get out of hand if there are many fields to authorize across many models. However, I think we should try out these solutions first before complicating Administrate's internal APIs in order to add more options that will rarely be used. Ultimately we may implement new APIs, but we'll benefit from trying other things first.

Would this solution work for you?

@sedubois
Copy link
Contributor Author

sedubois commented Feb 1, 2021

Thanks @pablobm for sharing your thoughts on this. In practice I probably would not be willing to make several copies of the same dashboard as it likely would quickly get out of hand and lead to more issues than it solves. It would also not cover all scenarios, such as the second and third points below.

Here are some use cases:

  • an editor should be able to view/modify most fields in a blog post, but only an admin should be able to change its publication status
  • some fields should be editable when modifying a resource, but not when creating it for the first time (e.g. because the model has some logic to auto-generate those values)
  • when nesting the info from within another model, some fields should not be shown (there are other issues about this such as Pass option to Field::HasOne to only display specific columns #1877)

NB: we already had a discussion in another issue about making dashboards inheritable/extensible (don't know where to find it however). It might not be an ideal solution but might be an improvement on the idea of just copying the dashboards.
NB2: I use Pundit and it's not clear what part of this problem should be handled at the policy level and what part should be dealt with through dashboard configuration.

@pablobm
Copy link
Collaborator

pablobm commented Feb 4, 2021

Good thinking. To add to the use cases, here's a generalisation of your second one:

  • Some fields should be editable only when the application is in a certain state. This could be the same model being edited, a different one, or even external factors like the day of the week.

The third one is probably the trickiest, due to the reasons I explained at #1877. However there may be a way to tackle the other two... here's a little hack I just tried.

  1. Copy Administrate template for app/views/administrate/application/_form.html.erb into your project, so that Rails picks your version instead of the original.
  2. In the template, find the block where it loops through the page.attributes, and add a condition.

For example, I just tried the following to hide the field "name" in the form for "customer":

  <% page.attributes.each do |attribute| -%>
    <% unless page.resource.is_a?(Customer) && attribute.attribute == :name %>
      <div class="field-unit field-unit--<%= attribute.html_class %> field-unit--<%= requireness(attribute) %>">
        <%= render_field attribute, f: f %>
      </div>
    <% end %>
  <% end -%>

If you want security, you'll also want to add checks at the update and create actions to ensure that nobody tries to post info that they are not allowed to post.

If you are using an authentication mechanism, you should be able to access current_user from there and defer to Pundit to make authorisation decisions.

So that's a first step! Now this could be improved...

  1. We could split Administrate's templates into more partials than we currently do. This way people could customise smaller parts of them with less risk of the templates changing in a new version and having to change the customisations too.
  2. We could add a hook to dashboards, which would be used in a condition like the one I propose above. Then users wouldn't need to customise their templates. The question here is: what information (parameters) should this hook have?

@sedubois
Copy link
Contributor Author

sedubois commented Feb 5, 2021

Thank you @pablobm, that is helpful. About point 2, I guess there needs to be access to the field, which itself knows about the attribute name, data, page, and resource, to which could be added the action. So we could define a method show_field?(field, action) in Administrate::ApplicationController similar to the existing show_action? which would always return true but could be overwritten with custom logic which could delegate to field-level Pundit policies if needed (e.g. to differentiate between users):

  <% page.attributes.select { |field| show_field?(field, action) }.each do |field| -%>
    <div class="field-unit field-unit--<%= field.html_class %> field-unit--<%= requireness(field) %>">
      <%= render_field field, f: f %>
    </div>
  <% end -%>

NB: in the code above I've used field instead of attribute to be consistent with what seemed to be their type (Administrate::Field::Base). Ideally page.attributes should be renamed page.fields?
NB2: in Administrate there is valid_action? and show_action?, not sure what's the difference....

@nickcharlton nickcharlton added the fields new fields, displaying and editing data label Feb 26, 2021
@pablobm
Copy link
Collaborator

pablobm commented Mar 25, 2021

Sorry for the delay. This is a difficult problem that requires some dedicated thinking...

Starting with your notate bene:

  1. I agree that page.fields would be more descriptive. I have been confused at that before.
  2. For the difference between valid_action? and show_action?, see my ongoing work to clarify them: Unify action checks #1941

As for where that hypothetical show_field? method should live. I'm not sure that it should be in the controller. Perhaps it should be in the dashboard? This is: Admininistrate::BaseDashboard#show_field?. One reason is that it would avoid mixing code affecting several different dashboards in the same method. Additionally, it may be necessary (I can't recall with certainty) when dealing with nested forms, like the ones provided by Field::HasOne

@pablobm
Copy link
Collaborator

pablobm commented Apr 16, 2021

See #1949 for another use case: loading fields depending on the application domain name.

@getaaron
Copy link

A nice API for this might be to provide skip: or only: options in the dashboard file, e.g.

customer: Field::HasOne.with_options(skip: [:name]),

or

customer: Field::HasOne.with_options(only: [:id, :favorite_color]),

In addition to an array, these could take a callback to return fields to show/hide, for example if you wanted to call Pundit or whatever:

customer: Field::HasOne.with_options(skip: ->(field) { Pundit.whatever ? [:name] : [] }),
customer: Field::HasOne.with_options(skip: ->(field) { Pundit.whatever ? [] : [:id, :favorite_color] }),

@DavidGeismarLtd
Copy link

Has there been any evolution on this ?

@Nitr
Copy link
Contributor

Nitr commented Aug 20, 2023

I did it like this.

module AttributesFilter
  extend ActiveSupport::Concern

  included do
    def form_attributes
      apply_attributes_filters(super, :form)
    end

    def show_page_attributes
      apply_attributes_filters(super, :show)
    end

    def collection_attributes
      apply_attributes_filters(super, :collection)
    end

    private
    def apply_attributes_filters(attributes, action)
      self.class.filters.each do |key, filters|
        if key.nil? || key == action
          filters.each do |filter|
            attributes = filter.call(attributes)
          end
        end
      end
      attributes
    end
  end

  class_methods do
    def filters
      @filters ||= {}
      @filters.dup
    end

    protected
    def filter_attributes(action = nil, &block)
      @filters ||= {}
      @filters[action] ||= []
      @filters[action] << block
    end
  end
end

class MyDashboard < ApplicationDashboard
  include AttributesFilter

  filter_attributes do |attributes|
     attributes -= %i[some_attribute] unless Current.user.admin?
     attributes
  end

  filter_attributes :collection do |attributes|
     # ....
  end

  filter_attributes :show do |attributes|
     # ....
  end
  
  filter_attributes :form do |attributes|
     # ....
  end
end

@sedubois
Copy link
Contributor Author

sedubois commented Jul 25, 2024

Thanks @Nitr, this workaround is helpful for filtering administrate dashboard attributes based on some runtime condition. It can be used to show the localized version of some attributes which can't be localized at the model level (we use Mobility for translations but there is no integration with ActiveStorage, and some attachments need to change per language).

class ContentDashboard < BaseDashboard
  include Admin::AttributesFilter

  ATTRIBUTE_TYPES = {
    # ...
    media_en: Field::ActiveStorage.with_options(...),
    media_fr: Field::ActiveStorage.with_options(...),
  }

  # ...
  
  SHOW_PAGE_ATTRIBUTES = %i[
    ...
    media_en
    media_fr
  ]

  # ...

  filter_attributes do |attributes|
    attributes -= %i[media_fr] unless I18n.locale == :fr
    attributes -= %i[media_en] unless I18n.locale == :en
    attributes
  end
end

NB: I had to update form_attributes as follows to avoid an error wrong number of arguments (given 1, expected 0) when editing a record.

module Admin::AttributesFilter
  extend ActiveSupport::Concern

  included do
    def form_attributes(action = nil)
      apply_attributes_filters(super, :form)
    end

    def show_page_attributes
      apply_attributes_filters(super, :show)
    end

    def collection_attributes
      apply_attributes_filters(super, :collection)
    end

    private
    def apply_attributes_filters(attributes, action)
      self.class.filters.each do |key, filters|
        if key.nil? || key == action
          filters.each do |filter|
            attributes = filter.call(attributes)
          end
        end
      end
      attributes
    end
  end

  class_methods do
    def filters
      @filters ||= {}
      @filters.dup
    end

    protected
    def filter_attributes(action = nil, &block)
      @filters ||= {}
      @filters[action] ||= []
      @filters[action] << block
    end
  end
end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug breakages in functionality that is implemented fields new fields, displaying and editing data fields-context
Projects
None yet
Development

No branches or pull requests

6 participants