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

removes access files overview permission #221

Open
wants to merge 3 commits into
base: 2.x
Choose a base branch
from

Conversation

markconroy
Copy link
Member

Closes #220

Removes the permission to access the files overview permission. It's probably only developers that need this for debugging things, so we can add that manually when needed, or add it to the admin role (when we have one).

===
Thanks to Big Blue Door for sponsoring my time to work on this.

Comment on lines 27 to 35
function _remove_access_files_overview_permission() {
$roles = Role::loadMultiple();
foreach ($roles as $role) {
if ($role->hasPermission('access files overview')) {
$role->revokePermission('access files overview');
$role->save();
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not so sure of this, if there are custom roles that have been assigned this then they will get deleted. Sometimes it is useful if there are files which for what ever reason are not yet in media. I'd limit this to localgov drupal provided roles, and I'd check if they have been altered from default. Noting the update hook also runs this.

@stephen-cox
Copy link
Member

As @andybroomfield points out - there are potential issues removing this permission for existing sites. Maybe just remove the update hook. We could always add a release note suggesting existing sites do this manually if it's important.

@finnlewis
Copy link
Member

Just discussing in Merge Tuesday.

Generally happy to remove the permission for fresh installs.

However, deploying an update hook that removes permissions from existing roles is perhaps a bit too far.

Perhaps we should remove the permission in the default install, and add instructions for people to clarify that they can remove the permission for certain roles.

@finnlewis
Copy link
Member

Note: would like to check where the permission 'access files overview' is being set to see if we can remove it there rather than here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hide the 'files' tab to non admins
4 participants