Flowpack / media-ui

The development repository for the new Neos media management interface
GNU General Public License v3.0
20 stars 16 forks source link

FEATURE: Implement privilege for hierarchical asset collections #233

Closed Sebobo closed 4 months ago

Sebobo commented 4 months ago

What I did

This change adds a new column to the collection „path“. Which allows a simple privilege check whether a user can access nested collections.

Resolves: #232

How I did it

Each collection has a new field "path" which is used to store the full hierarchical path of a collection. This path can be used with the newly added privileges to limit access to collections and contained assets.

How to verify it

Run the following commands to make the feature work

./flow doctrine:migrate
./flow assetcollections:updatepaths

And use&adapt the following policy for testing:

privilegeTargets:
  'Flowpack\Media\Ui\Security\Authorization\Privilege\ReadHierarchicalAssetCollectionPrivilege':
    'Some.Package:ReadSpecialAssetCollectionAndSubCollections':
      matcher: 'isInPath("/important-collections")'
  'Flowpack\Media\Ui\Security\Authorization\Privilege\ReadAssetPrivilege':
    'Some.Package:ReadSpecialAssets':
      matcher: 'isInCollectionPath("/important-collections")'

roles:
  'Neos.Neos:AbstractEditor':
    privileges:
      - privilegeTarget: 'Some.Package:ManageImportantCollections'
        permission: DENY
      - privilegeTarget: 'Some.Package:ReadSpecialAssets'
        permission: DENY

Remaining todos:

Sebobo commented 4 months ago

@lorenzulrich would be great if you could test this PR in your project.

I'm still thinking whether it's better to use the slugified path for the privilege matcher and path field or instead build the path from the persistence_object_identifier which would still work after renaming the collections. It would just be very ugly in the configuration.

lorenzulrich commented 4 months ago

@Sebobo I could test this successfully, thanks a lot!

I'm still thinking whether it's better to use the slugified path for the privilege matcher and path field or instead build the path from the persistence_object_identifier which would still work after renaming the collections. It would just be very ugly in the configuration.

I can live with the current state, while in general I'm more on the identifier side of things even though it's not nice to read. But as said, I can live the situation as it is.