Links to edit manifestation are shown even if user does not have required permissions

[damisul]

If user without edit_metadata permission clicks on it he gets redirected to home page with message רק בעלי הרשאת עריכה רשאים לגשת לדף זה

I believe we need to use same access check on read page to avoid those links to be rendered. I'd consider using Pundit gem for this, as described here: https://github.com/abartov/bybeconv/issues/171

[abartov]

Are you sure this happens? It does not happen in production, and there is explicit conditional code in the metadata box view that tests whether current_user.editor? or not. Perhaps you are shown this because you have logged in using the Developer button, which makes you an admin automatically?

[damisul]

Yes it surely happens. The problem is that in view we only check if user has editor bit:

  - if current_user && current_user.editor?

https://github.com/abartov/bybeconv/blob/17b1c9c46335dace3ff338f32083ca969a85740a/app/views/manifestation/_metadata.html.haml#L3

But in corresponding action we additionally check that user has specific flags :

  before_action only: [:list, :show, :remove_link, :edit_metadata, :add_aboutnesses] do |c| c.require_editor('edit_catalog') end
  before_action only: [:edit, :update] do |c| c.require_editor(['edit_catalog', 'conversion_verification', 'handle_proofs']) end

see https://github.com/abartov/bybeconv/blob/17b1c9c46335dace3ff338f32083ca969a85740a/app/controllers/manifestation_controller.rb#L6

Benefit of framwork like Pundit is that it will allow to write clear rules like ManifestationPolicy.edit_metadata?(manifestation)

and we can reuse such checks both in view and controller. So if some rules need to be updated we'll update both places at the same time. And it has some additional helper methods to simplify access checks.

[abartov]

Yeah, refactoring with Pundit may help tidy up, but not a priority for now.

I understand the access checks are not perfectly synced about the editor bits, but I still don't understand when a non-editor would see these links.

[damisul]

Ok, I agree, as those links are not shown to regular users, this is not urgent. We can surely postpone it.