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

Add permission for managing files to the Editor role by default. #4961

Closed
jenlampton opened this issue Feb 21, 2021 · 11 comments · Fixed by backdrop/backdrop#4420
Closed

Add permission for managing files to the Editor role by default. #4961

jenlampton opened this issue Feb 21, 2021 · 11 comments · Fixed by backdrop/backdrop#4420

Comments

@jenlampton
Copy link
Member

I am loving the new editor role, and the out-of-box permissions they give me for free on new sites. I would also love it if my editors had permissions to manage files.

These are the permissions I think we should add for editors:

  • Access the manage files overview
  • Add and upload new files
  • View own files
@stpaultim
Copy link
Member

Added that to this list: #4339

I think this is reasonable and makes sense.

@stpaultim
Copy link
Member

I created a PR. Needs testing and code review. Should be pretty easy.

This PR may conflict with PR in #6078

@klonos
Copy link
Member

klonos commented May 1, 2023

@stpaultim I've left a comment in the PR re the PHPCS complaint 😉 ...switch this back to "needs code review" once tests have passed (hopefully 🤞🏼).

@klonos klonos modified the milestones: 1.24.3, 1.25.1 Jun 6, 2023
@quicksketch quicksketch modified the milestones: 1.25.1, 1.25.2 Jun 7, 2023
@quicksketch quicksketch modified the milestones: 1.25.2, 1.26.1 Sep 15, 2023
@quicksketch quicksketch modified the milestones: 1.26.1, 1.26.2 Oct 6, 2023
@quicksketch quicksketch modified the milestones: 1.26.2, 1.26.3 Dec 1, 2023
@quicksketch quicksketch modified the milestones: 1.26.3, 1.26.4 Dec 20, 2023
@stpaultim
Copy link
Member

This PR has been updated and there should be a test sandbox.

I think we just need one or two more folks to chime in on whether or not this change makes sense as a default setting?

Atten: @olafgrabienski, @klonos, @kiamlaluno, @laryn, @Wylbur ?

@kiamlaluno
Copy link
Member

kiamlaluno commented Dec 27, 2023

Since the Editor role seems to be for moderating content, I would also give the following permissions:

  • View own private files
  • View private files
  • View files
  • Manage or replace any file

The role already has the following permissions. Giving the permission to replace any file or see any private file seems "coherent."

  • View any unpublished content
  • Delete content revisions

@argiepiano
Copy link

argiepiano commented Dec 27, 2023

@stpaultim, I think that basic set of permissions makes sense - it's a safe approach, since we are not giving the editor the ability to delete any file.

However, the new permissions lead to some inconsistencies that mostly should be addressed in follow-ups. I tested by logging in as editor, after giving the roles these 3 permissions (Access the manage files overview, Add and upload new files, View own files).

  1. As editor, with these permissions, I can add files by going to file/add. 👍🏽
  2. As editor, I can see the file overview table at admin/content/files 👍🏽
  3. As editor I can now upload files (i.e. create file entities) at file/add 👍🏽
  4. But, in the second step of file adding, I can actually select "Private local files served by Backdrop" for the file destination 🤔 This is strange, given that I won't be actually be able to access that file entity's page (available file/FID). So, actually, right after completing the file upload process, I get "Access denied" for the file I just uploaded! (WTF moment) 👎🏽 .

So, I would think that, now that the editor will be able to see the file overview table, we should:

  1. Open another issue to improve the file overview table
  2. Open another issue to not show the "Private" destination when the user doesn't have "View private files" permission
  3. Unfortunately, there is no "Delete own files" permission - possibly open another issue to add this permission

And, we should possibly consider adding at least "View files" and "View own private files" (to be on the safer side) from the list @kiamlaluno posted above.

@stpaultim
Copy link
Member

stpaultim commented Dec 30, 2023

* View own private files
* View private files
* View files
* Manage or replace any file

I added the following to this PR:

  • View Files
  • View Own Private Files

These two seem pretty sensible and I think we have agreement from several people on them. I'm hesitant to add the other two permissions, without more discussion.

@stpaultim
Copy link
Member

Revised version of PR adds these permissions:

'access file overview',
'create files',
'view own private files',
'view own files',
'view files'

RTBC anyone?

@kiamlaluno
Copy link
Member

It covers the permissions I suggested, so it works for me.

I would still wait and see if there are objections from other people, though. I know that missing permissions can still be added from the user interface, for sites that need them, but I would rather change the permissions once in the right way, instead of getting different issues to get the right permissions for a single role.

@stpaultim
Copy link
Member

Let's bring this up at the next Design/UX meeting to see if anyone thinks we are missing anything or got anything wrong. If we have consensus there, we can push this forward. But, I agree that having a few more eyes on this issue would be helpful.

@quicksketch
Copy link
Member

PR looks good to me too. Let's put this into 1.28.0 only since it's a difference that impacts expected permissions. I rebased the PR and all tests are passing.

Thanks @stpaultim, @jenlampton, @klonos, @docwilmot, @kiamlaluno, and @argiepiano! I merged backdrop/backdrop#4420 into 1.x for 1.28.0.

@quicksketch quicksketch modified the milestones: 1.27.2, 1.28.0 Apr 28, 2024
@jenlampton jenlampton changed the title [UX] Editors should have permission to manage files Add permission for managing files to the Editor role by default. May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants