• This site uses cookies. By continuing to use this site, you are agreeing to our use of cookies. Learn more.

Implemented [Developer tool] Add a new Abstract class to the Permissions system; AbstractFlatPermissions

DragonByte Tech

Well-known member
#1
At the moment, it is only possible to quickly create custom permissions for things that use a tree structure, such as categories or nodes. There's plenty of reasons why developers may want to create per-entry permissions for things that aren't trees.

Use case: I'm creating an eCommerce tool that will allow admins to set up coupons to provide discounts for products. However, admins may wish to restrict the coupon to only specific users, or only specific user groups.
I had to create a new macro to list & edit permissions (as you would expect).

Nothing has to change on your end to support this. This requires absolutely no modification of existing XenForo code, and is fully stand-alone.

Tests ran:
  • Private toggle
  • Per-usergroup permission, add custom / set back to inherit
  • Per-user permission, add custom / set back to inherit
  • Analysis (private on/off, UG on/off, per-user on/off)
I've found it working in all cases.

It is my belief that adding this to the core would provide a significant benefit to 3rd party developers who would want to implement custom per-item permissions for things that are not tree structures, while also enabling the XF core team to do the same.

In short, here's my proposal for a new abstract permission class:

PHP:
<?php

namespace XF\Permission;

use XF\Mvc\Entity\Entity;

/**
* Class AbstractFlatPermissions
*
* @package XF\Permission
*/
abstract class AbstractFlatPermissions extends AbstractContentPermissions
{
    /**
     * @var \XF\Mvc\Entity\ArrayCollection
     */
    protected $entries;
   
    /**
     * @var string
     */
    protected $repoIdentifier = '';
   
    /**
     *
     */
    protected function setupBuildTypeData()
    {
        $entryRepo = $this->builder->em()->getRepository($this->repoIdentifier);
        $this->entries = $entryRepo->findEntriesForPermissionList()->fetch();
    }

    /**
     * @param \XF\Entity\PermissionCombination $combination
     * @param array $basePerms
     *
     * @return array
     */
    public function rebuildCombination(\XF\Entity\PermissionCombination $combination, array $basePerms)
    {
        $entryIds = $this->entries->keys();
        if (!$entryIds)
        {
            return [];
        }

        $basePerms = $this->adjustBasePermissionAllows($basePerms);

        $built = [];
        foreach ($entryIds AS $entryId)
        {
            $built += $this->buildForContent($entryId, $combination->user_group_list, $combination->user_id, $basePerms);
        }
        $this->writeBuiltCombination($combination, $built);
    }

    /**
     * @param $contentId
     * @param array $userGroupIds
     * @param int $userId
     * @param array $basePerms
     *
     * @return array
     */
    protected function buildForContent($contentId, array $userGroupIds, $userId = 0, array $basePerms)
    {
        $sets = $this->getApplicablePermissionSets($contentId, $userGroupIds, $userId);
        $childPerms = $this->builder->calculatePermissions($sets, $this->permissionsGrouped, $basePerms);

        $calculated = $this->builder->applyPermissionDependencies($childPerms, $this->permissionsGrouped);
        $finalPerms = $this->builder->finalizePermissionValues($calculated);

        $output = [];
        $output[$contentId] = $finalPerms;

        return $output;
    }

    /**
     * @param \XF\Entity\PermissionCombination $combination
     * @param $contentId
     * @param array $basePerms
     * @param array $baseIntermediates
     *
     * @return array
     */
    public function analyzeCombination(
        \XF\Entity\PermissionCombination $combination, $contentId, array $basePerms, array $baseIntermediates
    )
    {
        $groupIds = $combination->user_group_list;
        $userId = $combination->user_id;

        $intermediates = $baseIntermediates;
        $permissions = $basePerms;
        $dependChanges = [];

        $titles = $this->getAnalysisContentPairs();

        $permissions = $this->adjustBasePermissionAllows($permissions);

        $sets = $this->getApplicablePermissionSets($contentId, $groupIds, $userId);
        $permissions = $this->builder->calculatePermissions($sets, $this->permissionsGrouped, $permissions);

        $calculated = $this->builder->applyPermissionDependencies(
            $permissions, $this->permissionsGrouped, $dependChanges
        );
        $finalPerms = $this->builder->finalizePermissionValues($calculated);

        $thisIntermediates = $this->builder->collectIntermediates(
            $combination, $permissions, $sets, $contentId, $titles[$contentId]
        );
        $intermediates = $this->builder->pushIntermediates($intermediates, $thisIntermediates);

        return $this->builder->getFinalAnalysis($finalPerms, $intermediates, $dependChanges);
    }

    /**
     * @return array
     */
    public function getAnalysisContentPairs()
    {
        $pairs = [];
        foreach ($this->entries AS $id => $value)
        {
            $pairs[$id] = $this->getContentTitle($value);
        }

        return $pairs;
    }

    /**
     * @param Entity $entity
     *
     * @return mixed|null
     */
    public function getContentTitle(Entity $entity)
    {
        return $entity->title;
    }
}
If anyone is reading this and wish to use this code in their own projects, they are of course free to do so :)
(Just remember to change the namespace and import the AbstractContentPermissions class.)


Fillip
 

DragonByte Tech

Well-known member
#2
Because of the edit limit (why tho :() I can't edit the OP, here's an update to the AbstractFlatPermissions. I didn't realise the permission builder would include the permission group as well, causing front-end checks to fail even though analysis checks and all the above checks succeeded.

Changes are:
  • Two abstract method definitions from TreeContentPermissions
  • Changes to analyzeCombination and buildForContent to call these abstract methods
Front-end checks have now also been completed.

PHP:
<?php

namespace XF\Permission;

use XF\Mvc\Entity\Entity;
use XF\Permission\AbstractContentPermissions;

/**
 * Class AbstractFlatPermissions
 *
 * @package XF\Permission
 */
abstract class AbstractFlatPermissions extends AbstractContentPermissions
{
    /**
     * @var \XF\Mvc\Entity\ArrayCollection
     */
    protected $entries;
    
    /**
     * @var string
     */
    protected $repoIdentifier = '';
    
    abstract protected function getFinalPerms($contentId, array $calculated, array &$childPerms);
    abstract protected function getFinalAnalysisPerms($contentId, array $calculated, array &$childPerms);
    
    /**
     *
     */
    protected function setupBuildTypeData()
    {
        $entryRepo = $this->builder->em()->getRepository($this->repoIdentifier);
        $this->entries = $entryRepo->findEntriesForPermissionList()->fetch();
    }

    /**
     * @param \XF\Entity\PermissionCombination $combination
     * @param array $basePerms
     *
     * @return array
     */
    public function rebuildCombination(\XF\Entity\PermissionCombination $combination, array $basePerms)
    {
        $entryIds = $this->entries->keys();
        if (!$entryIds)
        {
            return [];
        }

        $basePerms = $this->adjustBasePermissionAllows($basePerms);

        $built = [];
        foreach ($entryIds AS $entryId)
        {
            $built += $this->buildForContent($entryId, $combination->user_group_list, $combination->user_id, $basePerms);
        }
        $this->writeBuiltCombination($combination, $built);
    }

    /**
     * @param $contentId
     * @param array $userGroupIds
     * @param int $userId
     * @param array $basePerms
     *
     * @return array
     */
    protected function buildForContent($contentId, array $userGroupIds, $userId = 0, array $basePerms)
    {
        $sets = $this->getApplicablePermissionSets($contentId, $userGroupIds, $userId);
        $childPerms = $this->builder->calculatePermissions($sets, $this->permissionsGrouped, $basePerms);

        $calculated = $this->builder->applyPermissionDependencies($childPerms, $this->permissionsGrouped);
        $finalPerms = $this->getFinalPerms($contentId, $calculated, $childPerms);

        $output = [];
        $output[$contentId] = $finalPerms;

        return $output;
    }

    /**
     * @param \XF\Entity\PermissionCombination $combination
     * @param $contentId
     * @param array $basePerms
     * @param array $baseIntermediates
     *
     * @return array
     */
    public function analyzeCombination(
        \XF\Entity\PermissionCombination $combination, $contentId, array $basePerms, array $baseIntermediates
    )
    {
        $groupIds = $combination->user_group_list;
        $userId = $combination->user_id;

        $intermediates = $baseIntermediates;
        $permissions = $basePerms;
        $dependChanges = [];

        $titles = $this->getAnalysisContentPairs();

        $permissions = $this->adjustBasePermissionAllows($permissions);

        $sets = $this->getApplicablePermissionSets($contentId, $groupIds, $userId);
        $permissions = $this->builder->calculatePermissions($sets, $this->permissionsGrouped, $permissions);

        $calculated = $this->builder->applyPermissionDependencies(
            $permissions, $this->permissionsGrouped, $dependChanges
        );
        $finalPerms = $this->getFinalAnalysisPerms($contentId, $calculated, $permissions);

        $thisIntermediates = $this->builder->collectIntermediates(
            $combination, $permissions, $sets, $contentId, $titles[$contentId]
        );
        $intermediates = $this->builder->pushIntermediates($intermediates, $thisIntermediates);

        return $this->builder->getFinalAnalysis($finalPerms, $intermediates, $dependChanges);
    }

    /**
     * @return array
     */
    public function getAnalysisContentPairs()
    {
        $pairs = [];
        foreach ($this->entries AS $id => $value)
        {
            $pairs[$id] = $this->getContentTitle($value);
        }

        return $pairs;
    }

    /**
     * @param Entity $entity
     *
     * @return mixed|null
     */
    public function getContentTitle(Entity $entity)
    {
        return $entity->title;
    }
}

Fillip
 

DragonByte Tech

Well-known member
#3
Another element to this is a new Behavior for rebuilding permissions:

PHP:
<?php

namespace XF\Behavior;

use XF\Mvc\Entity\Behavior;

/**
 * Class PermissionRebuildable
 *
 * @package XF\Behavior
 */
class PermissionRebuildable extends Behavior
{
    /**
     * @return array
     */
    protected function getDefaultConfig()
    {
        return [
            'permissionContentType' => null
        ];
    }
    
    /**
     * @return array
     */
    protected function getDefaultOptions()
    {
        return [
            'rebuildCache' => true,
        ];
    }
    
    /**
     *
     */
    public function postSave()
    {
        if ($this->getOption('rebuildCache'))
        {
            if (
                $this->config['permissionContentType']
                && ($this->entity->isInsert())
            )
            {
                $this->app()->jobManager()->enqueueUnique('permissionRebuild', 'XF:PermissionRebuild');
            }
        }
    }
    
    /**
     *
     */
    public function postDelete()
    {
        if ($this->getOption('rebuildCache'))
        {
            if ($this->config['permissionContentType'])
            {
                $this->entity->db()->delete(
                    'xf_permission_entry_content',
                    'content_type = ? AND content_id = ?',
                    [$this->config['permissionContentType'], $this->id()]
                );

                $this->app()->jobManager()->enqueueUnique('permissionRebuild', 'XF:PermissionRebuild');
            }
        }
    }
}

Fillip
 

Chris D

XenForo developer
Staff member
#4
Because of the edit limit (why tho :() I can't edit the OP, here's an update to the AbstractFlatPermissions. I didn't realise the permission builder would include the permission group as well, causing front-end checks to fail even though analysis checks and all the above checks succeeded.

Changes are:
  • Two abstract method definitions from TreeContentPermissions
  • Changes to analyzeCombination and buildForContent to call these abstract methods
Front-end checks have now also been completed.

PHP:
<?php

namespace XF\Permission;

use XF\Mvc\Entity\Entity;
use XF\Permission\AbstractContentPermissions;

/**
* Class AbstractFlatPermissions
*
* @package XF\Permission
*/
abstract class AbstractFlatPermissions extends AbstractContentPermissions
{
    /**
     * @var \XF\Mvc\Entity\ArrayCollection
     */
    protected $entries;
   
    /**
     * @var string
     */
    protected $repoIdentifier = '';
   
    abstract protected function getFinalPerms($contentId, array $calculated, array &$childPerms);
    abstract protected function getFinalAnalysisPerms($contentId, array $calculated, array &$childPerms);
   
    /**
     *
     */
    protected function setupBuildTypeData()
    {
        $entryRepo = $this->builder->em()->getRepository($this->repoIdentifier);
        $this->entries = $entryRepo->findEntriesForPermissionList()->fetch();
    }

    /**
     * @param \XF\Entity\PermissionCombination $combination
     * @param array $basePerms
     *
     * @return array
     */
    public function rebuildCombination(\XF\Entity\PermissionCombination $combination, array $basePerms)
    {
        $entryIds = $this->entries->keys();
        if (!$entryIds)
        {
            return [];
        }

        $basePerms = $this->adjustBasePermissionAllows($basePerms);

        $built = [];
        foreach ($entryIds AS $entryId)
        {
            $built += $this->buildForContent($entryId, $combination->user_group_list, $combination->user_id, $basePerms);
        }
        $this->writeBuiltCombination($combination, $built);
    }

    /**
     * @param $contentId
     * @param array $userGroupIds
     * @param int $userId
     * @param array $basePerms
     *
     * @return array
     */
    protected function buildForContent($contentId, array $userGroupIds, $userId = 0, array $basePerms)
    {
        $sets = $this->getApplicablePermissionSets($contentId, $userGroupIds, $userId);
        $childPerms = $this->builder->calculatePermissions($sets, $this->permissionsGrouped, $basePerms);

        $calculated = $this->builder->applyPermissionDependencies($childPerms, $this->permissionsGrouped);
        $finalPerms = $this->getFinalPerms($contentId, $calculated, $childPerms);

        $output = [];
        $output[$contentId] = $finalPerms;

        return $output;
    }

    /**
     * @param \XF\Entity\PermissionCombination $combination
     * @param $contentId
     * @param array $basePerms
     * @param array $baseIntermediates
     *
     * @return array
     */
    public function analyzeCombination(
        \XF\Entity\PermissionCombination $combination, $contentId, array $basePerms, array $baseIntermediates
    )
    {
        $groupIds = $combination->user_group_list;
        $userId = $combination->user_id;

        $intermediates = $baseIntermediates;
        $permissions = $basePerms;
        $dependChanges = [];

        $titles = $this->getAnalysisContentPairs();

        $permissions = $this->adjustBasePermissionAllows($permissions);

        $sets = $this->getApplicablePermissionSets($contentId, $groupIds, $userId);
        $permissions = $this->builder->calculatePermissions($sets, $this->permissionsGrouped, $permissions);

        $calculated = $this->builder->applyPermissionDependencies(
            $permissions, $this->permissionsGrouped, $dependChanges
        );
        $finalPerms = $this->getFinalAnalysisPerms($contentId, $calculated, $permissions);

        $thisIntermediates = $this->builder->collectIntermediates(
            $combination, $permissions, $sets, $contentId, $titles[$contentId]
        );
        $intermediates = $this->builder->pushIntermediates($intermediates, $thisIntermediates);

        return $this->builder->getFinalAnalysis($finalPerms, $intermediates, $dependChanges);
    }

    /**
     * @return array
     */
    public function getAnalysisContentPairs()
    {
        $pairs = [];
        foreach ($this->entries AS $id => $value)
        {
            $pairs[$id] = $this->getContentTitle($value);
        }

        return $pairs;
    }

    /**
     * @param Entity $entity
     *
     * @return mixed|null
     */
    public function getContentTitle(Entity $entity)
    {
        return $entity->title;
    }
}

Fillip
Implemented (and the Behavior).
 

Chris D

XenForo developer
Staff member
#5
I think we should make a change here.

I don't think we should rely on a user having to know to populate a class property with an important value when we can just simply add an abstract method.

This is a bit of a breaking change, I guess, but seeing as we only released it yesterday and there might not be loads of use cases for it, I think we'll just about get away with it.
 

Attachments

Chris D

XenForo developer
Staff member
#6
Bleh, you can of course omit that superfluous return at the end of the rebuildCombination method, but note there are other changes there too. We expect this to be a void method so there's no need to return empty arrays anywhere and further the phpdoc shouldn't suggest that (I've just removed them for now, which is slightly worse than having wrong docs but we'll php doc all of this at some point :) )
 

Chris D

XenForo developer
Staff member
#7
Actually, I'm still not happy with this.
Diff:
Index: src/XF/Permission/AbstractFlatPermissions.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- src/XF/Permission/AbstractFlatPermissions.php    (revision 5a315d60325d987d2e33b22ada50e77954ee119c)
+++ src/XF/Permission/AbstractFlatPermissions.php    (date 1521138589000)
@@ -11,17 +11,14 @@
      */
     protected $entries;
 
-    abstract protected function getRepoIdentifier();
+    abstract public function getContentList();
+
     abstract protected function getFinalPerms($contentId, array $calculated, array &$childPerms);
     abstract protected function getFinalAnalysisPerms($contentId, array $calculated, array &$childPerms);
 
-    /**
-     *
-     */
     protected function setupBuildTypeData()
     {
-        $entryRepo = $this->builder->em()->getRepository($this->getRepoIdentifier());
-        $this->entries = $entryRepo->findEntriesForPermissionList()->fetch();
+        $this->entries = $this->getContentList();
     }
 
     public function rebuildCombination(\XF\Entity\PermissionCombination $combination, array $basePerms)
I didn't realise at the time that as well as having to populate the repo ID you also had to make sure your repo had a specific method. Let's just follow a similar approach to the other permission helpers and call an abstract method which needs to be implemented to fetch the permission list data.
 

DragonByte Tech

Well-known member
#8
Has the 2.0.3 download been patched with these changes? I haven't downloaded it yet (taken this week off to recuperate after working on eCommerce for 3 months with no break :D) so if you've patched the live download I can have a look.

IIRC I designed FlatPermissions based off of the existing Tree files, hence why I used the identifier and whatnot. I'm guessing the reason why you let Tree get away with it is that other areas already depend on having a repository added, so it's not as big of an ask as FlatPermissions would be?


Fillip
 

Chris D

XenForo developer
Staff member
#9
Actually that’s another thing, this file should probably be called FlatContentPermissions rather than AbstractFlatPermissons.

It’s closer in functionality to TreeContentPermissions rather than the base Abstract class.

None of them though require a repo identifier to be passed in (unless we did it like that very early on and changed it).

We wouldn’t be patching the download file for something relatively minor like this so you might have to just use both diffs to update for now.

I think there’ll be another one shortly to action that name change.

Great work on this though. It may have been quite useful to me today when adding some new functionality for the future... 😉
 

DragonByte Tech

Well-known member
#10
Actually that’s another thing, this file should probably be called FlatContentPermissions rather than AbstractFlatPermissons.
Fair enough :)

I'll have a wee looksee at it just now and see if I can get it working. I'll look forward to blaming you when the per-product permissions in eCommerce break down 😜

Great work on this though. It may have been quite useful to me today when adding some new functionality for the future... 😉
Ooo, shiny 😄

There is a small trap when it comes to this system: The behaviour for rebuilding permissions does not run on save like it does in the AdminCP. I had to change the postSave method in the PermissionRebuildable behaviour to this:
PHP:
    public function postSave()
    {
        if (
            $this->config['permissionContentType']
            && $this->getOption('rebuildCache')
            && $this->entity->isInsert())
        {
            $this->app()->jobManager()->enqueueUnique('permissionRebuild', 'XF:PermissionRebuild');
            
            if (\XF::app()->get('app.classType') == 'Pub')
            {
                // Public doesn't immediately run this
                $this->app()->jobManager()->runUnique('permissionRebuild', 2);
            }
        }
    }
Otherwise, this check would fail because the content permissions hadn't been updated:
PHP:
        if (!$product->canView())
        {
            return $this->redirect($this->buildLink('dbtech-ecommerce/categories', $category, ['pending_approval' => 1]));
        }
        
        return $this->redirect($this->buildLink('dbtech-ecommerce', $product));
Thoughts?


Fillip
 

Chris D

XenForo developer
Staff member
#11
I don't think we'd generally recommend having a system to handle editing permissions and therefore rebuilding outside of the Admin CP so you'd probably want to address that as a class extension to the Behavior or doing something in the entity or your public controller to trigger that.

As for the name change, I'm sure you'd just do this manually, but here's the diff anyway :)
Diff:
Index: src/XF/Permission/AbstractFlatPermissions.php
===================================================================
--- src/XF/Permission/AbstractFlatPermissions.php    (revision 54b057c153c55e6918d963c6b6d5f1dcf8852be2)
+++ src/XF/Permission/FlatContentPermissions.php    (date 1521142918000)
@@ -4,7 +4,7 @@
 
 use XF\Mvc\Entity\Entity;
 
-abstract class AbstractFlatPermissions extends AbstractContentPermissions
+abstract class FlatContentPermissions extends AbstractContentPermissions
 {
     /**
      * @var \XF\Mvc\Entity\ArrayCollection
 

DragonByte Tech

Well-known member
#12
I don't think we'd generally recommend having a system to handle editing permissions and therefore rebuilding outside of the Admin CP
It's not editing permissions, it's creating a new piece of content that uses content permissions :)

In this particular case, creating new products from the front-end.

I've applied the two diff files now as well as done the name change manually, I'll update the eCommerce code and get back to you if I run into any problems :)


Fillip
 

DragonByte Tech

Well-known member
#13
I don't think we'd generally recommend having a system to handle editing permissions and therefore rebuilding outside of the Admin CP so you'd probably want to address that as a class extension to the Behavior or doing something in the entity or your public controller to trigger that.
PHP:
    public function postSave()
    {
        parent::postSave();
        
        if (
            $this->config['permissionContentType']
            && $this->getOption('rebuildCache')
            && $this->entity->isInsert()
            && \XF::app()->get('app.classType') == 'Pub'
        )
        {
            // Public doesn't immediately run this
            $this->app()->jobManager()->runUnique('permissionRebuild', 2);
        }
    }
Appears to have done the trick :)

I still think something should be changed in order to allow use cases where permission-based content can also be created via the front-end without hacks, but for me personally I don't mind a quick wee extension :)

Other than that, everything else seems to work fine.

Can't say the same for installing the XF Importers as even after upgrade it still says I need XF 2.0.3, but that's for a separate thread if one hasn't been created already :)


Fillip
 

Chris D

XenForo developer
Staff member
#15
I think it's fair to say the entire underlying framework is more sane generally for permissions. As useful as this class is, it doesn't add anything that wasn't already possible, but certainly anything we can add which is reusable for us or add-on developers is a great addition.
 
Top