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

ENH add optional SiteConfig root level permissions #579

Conversation

andrewandante
Copy link
Contributor

@andrewandante andrewandante commented Oct 20, 2023

Fixes #113

Offers a SiteConfig-based approach to Root file permissions, in a similar vein to Pages.

Aims to have the same starting point as the current FileDefaultPermissions - that is, view by anyone, edit only by those with the Permission File::EDIT_ALL.

Can be configured with the following yaml (caveat, maybe not all that is needed, IDK):

---
Name: app-assetspermissions
After:
 - assetspermissions
---
SilverStripe\Core\Injector\Injector:
  SilverStripe\Security\PermissionChecker.file:
    class: SilverStripe\Security\InheritedPermissions
    constructor:
      BaseClass: SilverStripe\Assets\File
      CacheService: '%$Psr\SimpleCache\CacheInterface.InheritedPermissions'
    properties:
      DefaultPermissions: '%$SilverStripe\Assets\SiteConfigFilePermissions'
      GlobalEditPermissions:
        - CMS_ACCESS
  SilverStripe\Security\InheritedPermissionFlusher:
    properties:
      Services:
        - '%$SilverStripe\Security\PermissionChecker.file'
SilverStripe\SiteConfig\SiteConfig:
  extensions:
    - SilverStripe\Assets\RootLevelAccessSiteConfigExtension

TODO

  • probably remove SiteConfig as a dependency, this shouldn't be invoked unless it is installed
  • hide things properly on the page (doing that thing where I need to adjust some React code or all the options are open
  • update Readme/Docs to offer this option

@andrewandante andrewandante force-pushed the ENH/add_root_level_asset_permissions branch from e77c0d4 to 2529530 Compare October 20, 2023 03:15
@GuySartorelli
Copy link
Member

GuySartorelli commented Oct 20, 2023

I'm not sure if SiteConfig is the right place for this - it would make more sense to me if instead there was a way to configure this on the root 'folder' in asset admin itself. There's no reason for this to be tied to SiteConfig other than that's where sitetree handles its root permissions, which makes sense in that context because:

  1. SiteConfig is already required by the CMS module
  2. For pages, those permissions are about access to the site

Asset admin shouldnt have any dependency on SiteConfig, and access to files I think is sufficiently separate from site access that it should be contained withing the asset admin directly.

@andrewandante
Copy link
Contributor Author

Fair - from a user's perspective, I think that's where I'd expect it to be, but equally we can't put it there without SiteConfig, so I think in this case it should go somewhere else. Thinking here?

image

Could pop it out like the current "folder" Edit Form, with only a permissions "tab"?

@GuySartorelli
Copy link
Member

Yup, that'd make sense to me.
Or perhaps just add an edit icon next to "Files" like when you're editing a folder - since that's essentially what we're doing here.
edit button next to folder name
And then as you say, only have the permissions tab in the form.

@andrewandante
Copy link
Contributor Author

Oh yeah that's clean, I like that a lot. Now to figure out how to do it 😉

@GuySartorelli
Copy link
Member

In the meantime I'll close this PR - please create a new one when/if you have a way forward on the new approach.

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.

Root-level assets permissions missing from SiteConfig
2 participants