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

Hide invalid edit button #2814

Open
1 of 2 tasks
Macroz opened this issue Dec 21, 2021 · 4 comments · Fixed by #3126
Open
1 of 2 tasks

Hide invalid edit button #2814

Macroz opened this issue Dec 21, 2021 · 4 comments · Fixed by #3126

Comments

@Macroz
Copy link
Collaborator

Macroz commented Dec 21, 2021

We should hide the edit button from the organization owner if the catalogue item etc. is not theirs to edit. They can access the page and get an error from the API at the moment.

  • admin tables
  • details pages
@tjb
Copy link

tjb commented Jan 12, 2022

hello! i would be happy to take this on. I have some questions though that would need clarification.

disclaimer i am new to this open source project & clojure but want to learn!

After taking some time to dig into the roles within the system i noticed that there are :organization-owner and :owner roles. I am assuming that the :owner role is for when a catalogue item is created and designates that user as the creator or "owner" of that catalogue item (please correct me if i am wrong and misunderstanding!).

In the current implementation of the catalogue item list all buttons are shown which are View, Edit, Enable, and Archive. This is due to a check occurring that allows these buttons to be shown if the user is has :organization-owner (regardless of what organization they are an owner of) or :owner shown here on this line

Going off the initial spec of hiding the Edit button from users who are not owner's of that catalogue item, should we also hide Enable and Archive from those users as well? If not, should any organization owner have the ability to Enable or Archive a catalogue item? I would think a user can only do that iff the catalogue item belongs to the organization they are an owner of or if the user is the owner of that catalogue item.

I can achieve the intent result of hiding the Edit button by wrapping the edit button with roles/show-when #{:owner} assuming :owner equates to catalogue item creator but wanted to see if hiding the other buttons were necessary as well.

please correct me if i am wrong in any of my assumptions above. i appreciate the guidance and look forward to making a pull request for this.

@Macroz
Copy link
Collaborator Author

Macroz commented Jan 14, 2022

Hello!

So an :owner is like the superuser of the entire instance. They can set it up as they like. An :organization-owner is a delegated owner of a part of the instance, an organization. Both are able to view and edit the various domain objects. However, the :organization-owner should only be able to modify what belongs in their own organization. Now we have not bothered to show the edit button with the accurate logic yet, but it would be a nice feature (of course it could end up too slow/complicated but we'll see).

As a side-note, the :handler user is able to view everything, but not modify. They are the people who handle applications but are not allowed to modify the domain objects (with the exception that they may need to create licenses that sometimes model custom contracts). In some organizations the same user can be an owner and a handler.

You are well on the right path. For the :organization-owner we should somehow check that the domain object organization matches the user's "owned organizations". You may find the logged-in user's "owned organizations" from the :rems.config/owned-organizations re-frame subscription. This could be checked against each objects' organization to see if they match and hide editing actions if not.

A pure role-level check is not going to be enough for the :organization-owner as they are rather coarse-grained. That is meant for things that are possible for all organization owners, but in this case the permission is more fine-grained, i.e., it depends on the actual organizations involved. So some code is involved and a helper could be useful to sprinkle around the administration pages.

You are correct that it would be nice to have Enable and Archive there as well.

P.S. Here is a useful page that is autogenerated from the domain. It lists which sort of things are possible for which user. Unfortunately these are only for application handling, and not for the domain objects. https://github.com/CSCfi/rems/blob/master/docs/application-permissions.md

@tjb
Copy link

tjb commented Jan 14, 2022

@Macroz thanks a ton for the detailed response to my, perhaps annoying, questions! I started to dig and work on a solution last night to check the if the user matches their owned organizations and will use your suggestion to look at :rems.config/owned-organizations. I will extend this for Enable and Archive as well.

I should be able to have a PR up by next week!

tjb added a commit to tjb/rems that referenced this issue Jan 15, 2022
tjb added a commit to tjb/rems that referenced this issue Jan 15, 2022
@Macroz
Copy link
Collaborator Author

Macroz commented Mar 24, 2023

I think we could still improve with the buttons on the details page so I have updated this issue and reopened it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Backlog
Development

Successfully merging a pull request may close this issue.

2 participants