beechit / fal_securedownload

An extension for TYPO3 CMS which adds secure downloads to FAL
36 stars 62 forks source link

CheckPermissions: rootline, resource groups only, group inheritance #244

Open haraldwitt opened 9 hours ago

haraldwitt commented 9 hours ago

Some issues in BeechIt\FalSecuredownload\Security\CheckPermissions->getPermissions(ResourceInterface $resource)

  1. Go up the rootline The break statement in the foreach loop should only be executed if $feGroups comtains at least one element. Otherwise the loop should continue until a group is found or the rootline ends.

  2. Only the file has resource group(s) assigned If a file resource has a fe_group assigned but not any folder in the rootline, $feGroups will obviousely be empty before and after ArrayUtility::keepItemsInArray(). In such a case the fe_groups of the file resource have to be overtaken directly.

  3. Take care of group inheritance? Imagine groups inside $feGroups are subgroups of other groups. So those other groups should also be granted access. So $feGroups should be enriched by these other groups. And this should be done for both - the $feGroups of the folder and for the file resource - before they are merged together with ArrayUtility::keepItemsInArray().

  4. Contradictory fe_groups? Assume the foreach loop ends with some groups inside $feGroups, but the fe_groups of the file resource does not fit to them. In this case $feGroups will be empty after keepItemsInArray() although there has been a secured folder and file. But an empty $feGroups means access for everyone. This is not good for the security. In such a case it would be better nobody has access. I purpose to take the current timestamp as fe_group in such a case. But: solrfal will kick out this nonexisting group. So I'll leave it as it is for now :-(

I'll fork the GitHub repository and create a pull request for issues 1-3.

Greetings Harald

haraldwitt commented 9 hours ago

And here ist the pull request: https://github.com/beechit/fal_securedownload/pull/245 I made 3 commits for the 3 points above. So everyone can retrace what I've done.