Uh oh!
There was an error while loading. Please reload this page.
feat: support hierarchical permission wildcards - #1327
Conversation
Add hierarchical wildcard matching for Shield permissions. - Support nested trailing wildcards like forum.posts.* - Support middle-segment wildcards like forum.*.create - Share wildcard matching between user and group permission checks - Document wildcard semantics and direct user wildcard assignment - Cover matcher behavior and public authorization paths Co-authored-by: bgeneto <bgeneto@duck.com> Co-authored-by: christianberkman <christianberkman@users.noreply.github.com> Signed-off-by: memleakd <121398829+memleakd@users.noreply.github.com>
michalsn
left a comment
There was a problem hiding this comment.
I would like to make wildcards simpler: trailing wildcards match children only, never the parent. So forum.posts.* would match forum.posts.create but NOT forum.posts.
The current "parent matching" feels like hidden privilege escalation. If someone sets up forum.posts.* thinking "all actions on posts", they probably don't expect it also grants the bare forum.posts scope - which could be used as a different kind of gate elsewhere in their app. Current behavior is non-intuitive to me.
datamweb
commented
May 29, 2026
@michalsn I agree, and I think this is the direction we should standardize on as a team. The overall goal of supporting deeper hierarchical wildcard permissions still seems valid, but I do not think trailing wildcards should also grant the parent scope itself. Allowing The clearer contract, in my view, is: 1- That keeps permission grants explicit, avoids hidden coupling between namespace-like labels and actual permission checks, and gives us simpler semantics to document and test consistently across both group-level and direct user permissions. So my preference would be to keep the feature, but adjust the trailing-wildcard semantics to follow this rule before merging. |
Signed-off-by: memleakd <121398829+memleakd@users.noreply.github.com>
There was a problem hiding this comment.
<?phpdeclare(strict_types=1);
/** * This file is part of CodeIgniter Shield. * * (c) CodeIgniter Foundation <admin@codeigniter.com> * * For the full copyright and license information, please view * the LICENSE file that was distributed with this source code. */namespaceCodeIgniter\Shield\Authorization;
/** * Matches permission grants against requested permission names for Shield authorization internals. */finalclass PermissionMatcher
{
/** * @param list<string> $grants */publicstaticfunctionmatches(string$permission, array$grants): bool
{
if (! self::isValid($permission)) {
returnfalse;
}
// Cache the exploded permission to prevent redundant string parsing in loops$permissionSegments = explode('.', $permission);
$permissionSegCount = count($permissionSegments);
foreach ($grantsas$grant) {
// Exact match first// If it matches exactly, it's inherently valid (since $permission is valid)// This skips the isValid() overhead entirely for exact matchesif ($grant === $permission) {
returntrue;
}
if (! self::isValid($grant)) {
continue;
}
if (str_contains($grant, '*') && self::matchesWildcardGrant($grant, $permissionSegments, $permissionSegCount)) {
returntrue;
}
}
returnfalse;
}
/** * @param list<string> $permissionSegments */privatestaticfunctionmatchesWildcardGrant(string$grant, array$permissionSegments, int$permissionSegCount): bool
{
$grantSegments = explode('.', $grant);
$grantSegCount = count($grantSegments);
// Check trailing wildcard efficientlyif ($grantSegments[$grantSegCount - 1] === '*') {
array_pop($grantSegments);
$grantSegCount--; // Decrement count after pop// Trailing wildcard matches children only, never the parent itself.return$permissionSegCount > $grantSegCount
&& self::segmentsMatch($grantSegments, $permissionSegments, $grantSegCount);
}
if ($permissionSegCount !== $grantSegCount) {
returnfalse;
}
returnself::segmentsMatch($grantSegments, $permissionSegments, $grantSegCount);
}
/** * @param list<string> $grantSegments * @param list<string> $permissionSegments */privatestaticfunctionsegmentsMatch(array$grantSegments, array$permissionSegments, int$matchLimitCount): bool
{
// Using "for" with a strict limit instead of array_slice() overheadfor ($i = 0; $i < $matchLimitCount; $i++) {
if ($grantSegments[$i] !== '*' && $grantSegments[$i] !== $permissionSegments[$i]) {
returnfalse;
}
}
returntrue;
}
privatestaticfunctionisValid(string$permission): bool
{
// Catch invalid starts right away before allocating an array.if ($permission === '' || str_starts_with($permission, '*')) {
returnfalse;
}
$segments = explode('.', $permission);
foreach ($segmentsas$segment) {
if ($segment === '' || ($segment !== '*' && str_contains($segment, '*'))) {
returnfalse;
}
}
returntrue;
}
}I was playing around with the code to see if we could squeeze out a bit more performance (mostly by avoiding array_slice overhead and optimizing the explode logic). I ran a benchmark test with 100,000 iterations, and the results were actually quite promising:
Running Benchmark (100000 iterations)...Original Code:Time: 488.31 msOptimized Code:Time: 395.79 msPerformance Improvement: 18.95%@memleakd What are your thoughts on this?
Thanks for digging into this and putting together the benchmark idea @datamweb. I played with it a bit more locally and compared the implementations: Click to see the optimized matcher class<?phpdeclare(strict_types=1);
/** * This file is part of CodeIgniter Shield. * * (c) CodeIgniter Foundation <admin@codeigniter.com> * * For the full copyright and license information, please view * the LICENSE file that was distributed with this source code. */namespaceCodeIgniter\Shield\Authorization;
/** * Matches permission grants against requested permission names for Shield authorization internals. */finalclass PermissionMatcher
{
/** * @param list<string> $grants */publicstaticfunctionmatches(string$permission, array$grants): bool
{
$permissionSegments = self::splitIfValid($permission);
if ($permissionSegments === null) {
returnfalse;
}
$permissionSegmentCount = count($permissionSegments);
foreach ($grantsas$grant) {
if ($grant === $permission) {
returntrue;
}
$wildcardPosition = strpos($grant, '*');
if ($wildcardPosition === false) {
continue;
}
if (
$wildcardPosition > 0
&& $wildcardPosition === strlen($grant) - 1
&& $grant[$wildcardPosition - 1] === '.'
) {
$prefix = substr($grant, 0, $wildcardPosition - 1);
if (self::isLiteral($prefix) && str_starts_with($permission, $prefix . '.')) {
returntrue;
}
continue;
}
$grantSegments = self::splitIfValid($grant);
if ($grantSegments === null) {
continue;
}
if (self::matchesWildcardGrant($grantSegments, $permissionSegments, $permissionSegmentCount)) {
returntrue;
}
}
returnfalse;
}
/** * @param list<string> $grantSegments * @param list<string> $permissionSegments */privatestaticfunctionmatchesWildcardGrant(array$grantSegments, array$permissionSegments, int$permissionSegmentCount): bool
{
$grantSegmentCount = count($grantSegments);
if ($grantSegments[$grantSegmentCount - 1] === '*') {
$grantSegmentCount--;
return$permissionSegmentCount > $grantSegmentCount
&& self::segmentsMatch($grantSegments, $permissionSegments, $grantSegmentCount);
}
return$permissionSegmentCount === $grantSegmentCount
&& self::segmentsMatch($grantSegments, $permissionSegments, $grantSegmentCount);
}
/** * @param list<string> $grantSegments * @param list<string> $permissionSegments */privatestaticfunctionsegmentsMatch(array$grantSegments, array$permissionSegments, int$segmentCount): bool
{
for ($index = 0; $index < $segmentCount; $index++) {
if ($grantSegments[$index] !== '*' && $grantSegments[$index] !== $permissionSegments[$index]) {
returnfalse;
}
}
returntrue;
}
/** * @return list<string>|null */privatestaticfunctionsplitIfValid(string$permission): ?array
{
if ($permission === '' || str_starts_with($permission, '*')) {
returnnull;
}
$segments = explode('.', $permission);
foreach ($segmentsas$segment) {
if ($segment === '' || ($segment !== '*' && str_contains($segment, '*'))) {
returnnull;
}
}
return$segments;
}
privatestaticfunctionisLiteral(string$permission): bool
{
return$permission !== ''
&& ! str_contains($permission, '*')
&& ! str_contains($permission, '..')
&& ! str_starts_with($permission, '.')
&& ! str_ends_with($permission, '.');
}
}
I used 100,000 iterations, same as your benchmark: XDEBUG_MODE=off php -d opcache.enable_cli=0 permission-matcher-compare.php 100000Here is one representative run:
I kept the direction you suggested, then adjusted it a bit further: the matcher now parses the requested permission once, avoids I also tried simplifying the class more, but those versions were slower in the trailing wildcard path, so I kept this as the best balance I could find between readability and performance. I have not committed this yet, so I’m happy to go either way. If the team thinks the performance gains are worth the slightly larger matcher, I can commit this version here. I'd appreciate your thoughts on the tradeoff. |
michalsn
commented
May 31, 2026
Thanks, these numbers look good. The absolute savings are small in application terms, but parsing the requested permission once and avoiding repeated allocations are sensible improvements for code that may run frequently. I'm open to the optimized version as long as the fast path remains straightforward and covered by the existing edge-case tests. Please commit it so we can review it. |
Signed-off-by: memleakd <121398829+memleakd@users.noreply.github.com>
memleakd
commented
May 31, 2026
Thanks @michalsn, that sounds good. I just pushed the optimized version. I kept the fast path limited to trailing wildcards, with the existing edge-case tests covering it. |
datamweb
left a comment
There was a problem hiding this comment.
While the new implementation introduces a bit more complexity, I believe the significant performance and efficiency gains are absolutely worth the trade-off in this context.
Uh oh!
There was an error while loading. Please reload this page.
Signed-off-by: memleakd <121398829+memleakd@users.noreply.github.com>
Uh oh!
There was an error while loading. Please reload this page.
Signed-off-by: memleakd <121398829+memleakd@users.noreply.github.com>
datamweb
commented
Jun 3, 2026
memleakd
commented
Jun 3, 2026
christianberkman
commented
Jun 4, 2026
@memleakd Thank you for finally making this happen. |
This PR intends to finalize the hierarchical permissions work from #1253 and #1309, which had stalled for some time.
Shield already supports wildcard permissions, but the current behavior is limited when permissions are organized into deeper namespaces.
In larger, real-world applications, permissions often grow beyond two segments. The flat/single-level wildcard behavior makes those permission sets harder to express and maintain cleanly, often requiring custom workarounds.
This PR makes wildcard permissions work with deeper structures while keeping the existing
$user->can()and$group->can()APIs unchanged.A trailing wildcard now matches descendant permissions:
'forum.posts.*'matches
forum.posts.createandforum.posts.comments.delete, but notforum.posts.Wildcards can also be used in the middle of a permission, so
forum.*.creatematchesforum.posts.createandforum.comments.create, but notforum.posts.comments.create.I tried to address the concerns and review feedback from the earlier attempts:
Config\AuthGroups::$permissionsbefore assignment;*must be a complete segment;*cannot be the first segment;*does not grant everything;$user->can()/$group->can()paths.I’d appreciate any feedback. Hopefully this can help move this long-standing and requested feature forward.
resolves#1253, resolves#1309,