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

Permissions update: Get rid of remaining checks on ROLE_ADMIN #2342

Closed
simongroenewolt opened this issue Jan 24, 2021 · 6 comments
Closed

Permissions update: Get rid of remaining checks on ROLE_ADMIN #2342

simongroenewolt opened this issue Jan 24, 2021 · 6 comments

Comments

@simongroenewolt
Copy link
Contributor

Searching in the Bolt sources leads to a couple of 'direct' uses of the ROLE_ADMIN role to check for access. This should be updated to a check against a (new?) permission listed in permissions.yaml to make these developer-configurable.

Below a list of controller classes that still need to be updated. I think most are easy to fix as long as someone decides what the name of the permission should be or which existing permission can be used.

ContentLocalizationController.php

  • /edit_locales/{id}

MediaController.php

  • /media/crawl/{location}

MediaEditController.php

  • /media/edit/{id}
  • /media/edit/{id}
  • /media/new

EmbedController.php

  • /embed

FileListingController.php

  • /list_files
@simongroenewolt
Copy link
Contributor Author

simongroenewolt commented Jan 24, 2021

/edit_locales/{id} is needed when editing multi-language entries. In think this means the permissions should match the contenttype's edit permission. You cannot switch the locale when creating new content, so 'create' permission isn't needed.

FIXED

@simongroenewolt
Copy link
Contributor Author

simongroenewolt commented Jan 24, 2021

MediaController.php: /media/crawl/{location} doesn't look like it is being used anywhere.

@simongroenewolt
Copy link
Contributor Author

simongroenewolt commented Jan 24, 2021

/media/edit/{id} is used to edit metadata of images etc. -> create separate permission I'd say.

I think /media/new is used to create a new crop of an existing image? @bobdenotter or @I-Valchev can you maybe explain this use? In my version the label of this action is 'edit attributes'.

FIXED by introducing permission media_edit

@simongroenewolt
Copy link
Contributor Author

simongroenewolt commented Jan 24, 2021

/embed allows you to get some data (title?) so you can embed a youtube (or other?) video, the call itself doesn't change anything, it just responds with a bit of json. -> create separate permission I'd say, but maybe don't call it 'embed' as that would suggest this permission is to set 'embed' fields, but that is dependent on the content-type permissions.

FIXED by introducing permission fetch_embed_data

@simongroenewolt
Copy link
Contributor Author

simongroenewolt commented Jan 24, 2021

/list_files -> pick previousely added files to link to content. Needed whenever you edit content, but we don't know the 'context' so I cannot check for a specific content-type permission -> add global permission, maybe managefiles:files_list, although that might suggest it is part of the 'managing' of the files instead of the choosing.

FIXED by introducing 3 permissions list_files:config, list_files:files, list_files:themes as the list files can be called for all 3 locations. I think only list_files:files is actually expected.

simongroenewolt added a commit to simongroenewolt/bolt-core that referenced this issue Jan 24, 2021
…check in the locales() method on 'view' permission. (reference: bolt#2342)
bobdenotter pushed a commit that referenced this issue Jan 29, 2021
…check in the locales() method on 'view' permission. (reference: #2342)
@simongroenewolt
Copy link
Contributor Author

Closing this issue: All the changes mentioned have been done and merged.

simongroenewolt added a commit to simongroenewolt/bolt-core that referenced this issue May 11, 2021
…check in the locales() method on 'view' permission. (reference: bolt#2342)
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

No branches or pull requests

1 participant