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

FEATURE: Implement privilege for hierarchical asset collections #233

Merged
merged 4 commits into from Mar 7, 2024

Conversation

Sebobo
Copy link
Member

@Sebobo Sebobo commented Feb 22, 2024

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:

  • Add migration for Postgres
  • Set path when collection is created
  • Fix Unittests

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
@Sebobo Sebobo added the Feature A new feature label Feb 22, 2024
@Sebobo Sebobo added this to the Goal - Extended permissions milestone Feb 22, 2024
@Sebobo Sebobo self-assigned this Feb 22, 2024
@Sebobo
Copy link
Member Author

Sebobo commented Feb 22, 2024

@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
Copy link
Contributor

@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.

@Sebobo Sebobo merged commit 34ab06a into main Mar 7, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature A new feature
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Extend ReadAssetPrivilege and ReadAssetCollectionPrivilege to work with nested collections
2 participants