Security policy wildcard support for methods/properties#3901
Security policy wildcard support for methods/properties#3901YSaxon wants to merge 8 commits intotwigphp:2.xfrom
Conversation
…are and would throw an error immediately anyways
fabpot
left a comment
There was a problem hiding this comment.
That will be for 3.x as 2.x EOL is reached.
I know I've merged your other PR in 2.x, but as it was security related, I accepted it
| * - Method/property can also be specified with a trailing wildcard to allow all methods/properties with a certain prefix, eg. `\DateTime => ['get*', ...]` in order to allow all methods/properties that start with `get`. | ||
| * | ||
| * @author Yaakov Saxon <ysaxon@gmail.com> | ||
| */ |
There was a problem hiding this comment.
Let's mark this class as@internal.
| */ | ||
| final class MemberMatcher | ||
| { | ||
| private $allowedMembers; |
There was a problem hiding this comment.
| private $allowedMembers; | |
| private array $allowedMembers; |
| final class MemberMatcher | ||
| { | ||
| private $allowedMembers; | ||
| private $cache = []; |
There was a problem hiding this comment.
| private $cache = []; | |
| private array $cache = []; |
|
|
||
| public function isAllowed($obj, string $member): bool | ||
| { | ||
| $cacheKey = get_class($obj) . "::" . $member; |
There was a problem hiding this comment.
| $cacheKey = get_class($obj) . "::" . $member; | |
| $cacheKey = get_class($obj).'::'.$member; |
| if ('*' === $class || $obj instanceof $class) { | ||
| foreach ($members as $allowedMember) { | ||
| if ('*' === $allowedMember) { | ||
| $this->cache[$cacheKey] = true; |
There was a problem hiding this comment.
| $this->cache[$cacheKey] = true; | |
| return $this->cache[$cacheKey]; |
Same below
|
|
||
| return true; | ||
| } | ||
| // if allowedMember ends with a *, check if the member starts with the allowedMember |
There was a problem hiding this comment.
Why do we only allow *to be at the end?
|
I forgot to mention that overall I'm not sold yet this is something I want in core. I very much prefer white-listing explicitly what is allowed. With |
|
Or maybe that's fine but we need to make it very clear in the docs that this should be used with caution (can you some docs for this new feature?). |
Allows for flexible wildcard support in allowedMethods and allowedProperties in SecurityPolicy.
* => ['foo',...]in order to allow those methods/properties for all classes.\Foo\Bar\Baz => '*'in order to allow all methods/properties for that class.\Foo\Bar\Baz => ['get*', ...]in order to allow all Baz methods/properties that start withget.Here are some real examples of security policy items that previously had to be hardcoded that can now be expressed with wildcards: