Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Support Enum as Security attribute/role#46006

Unanswered
sadikoff asked this question inQ&A
Apr 12, 2022· 6 comments· 19 replies
Discussion options

It can be nice to have the full support of Enums for security voters and maybe roles too. For now, in most places$attribute parameter is not type-hinted ormixed type is used depending on Symfony version, but
here

abstractprotectedfunctionsupports(string$attribute,mixed$subject):bool;

and here
abstractprotectedfunctionvoteOnAttribute(string$attribute,mixed$subject,TokenInterface$token):bool;

$attribute should be string, and multiple voter implementations requires attribute to bestring which not works if you pass Enum as attribute.

You must be logged in to vote

Replies: 6 comments 19 replies

Comment options

stof
Apr 12, 2022
Collaborator

Well, enums are meant to represent aclosed list of values. In the security system, the list of permission attributes is unknown. It is an open list. So enums are not suited for that.

You must be logged in to vote
5 replies
@sadikoff
Comment options

Yep, I got your thought, but what about inconsistency in parameters definition? I mean in some places$attributes aremixed and in somestring?

@tarlepp
Comment options

imho - roles - is closed lists.

@DylanKas
Comment options

It's rarely the case when building application with a complex or dynamic role system

@joshpme
Comment options

With match expressions now supporting exhaustive match hinting in some IDEs.

And bits of code like this common in custom voters.

protected function supports($attribute, $subject) {        $validChecks = [self::REVIEW, self::PAYMENT, self::REFUND, self::CANCEL, self::MODIFY, self::THANK];        if(!in_array($attribute, $validChecks)) {            return false;        }

Attributes allowing the support of enum values.

Supporting enums in voters is clear improvement.

@stof
Comment options

stofSep 24, 2024
Collaborator

@tarlepp in your specific project, it likely is a closed list. But insymfony/security-core, it is not.
Each project using Symfony will want to have theirown list of roles, not a closed list defined insymfony/security-core.

Comment options

I am using Roles as Enums, because it's finite list of values. But I find it very strange that up untilAccessDecisionManager::getVoters I can pass anything as$attributes. E.g.AuthorizationCheckerInterface->isGranted(AuthorizationEnum::ADMIN).

But when getting voter, it suddenly starts to work with only strings. At least acceptStringable objects.

Now I have to do->isGranted(AuthorizationEnum::ADMIN->value, AuthorizationEnum::ADMIN) just for my voter to be able to work with Enums.

You must be logged in to vote
1 reply
@stof
Comment options

stofSep 24, 2024
Collaborator

Accepting stringable won't help you, as enums cannot be stringable.

Comment options

I would like to get some clarification on this issue.

I would argue that most real life applications out there will have any form of constant holder for Roles / Permissions / similar tokens. When taking into considerations that database also holds values like this, then having them stored as an enum makes a hell of a lot of sense.

I see absolutely no harm in supporting backedEnums for the isGranted functionality.

NOT doing so, honestly just promotes userland code errors. Why the hard stance against this?

You must be logged in to vote
2 replies
@stof
Comment options

stofSep 24, 2024
Collaborator

Enums provide a benefit to avoid errors when you can use them as the type of parameters to restrict things. But the AuthorizationCheckerInterface cannot use such application-specific enum as its parameter type, as it cannot depend on it.

PHP enums are representing a type with aclosed list of values. But as far assymfony/security-core is concerned, permissions are anopen list (which is what allows you to define your own permissions in your application) and so enums are not the right fit for that.

@Menelion
Comment options

@stof Christophe, sorry, but I still don't understand your point. I like the DRY principle (as many do, I bet). So in my app I have a set of permissions like that:

enum ProjectPermission:string{case View ='view';case MarkAsDone ='markAsDone';}

To me it's a closed set of options, if I need more, I'll add, but it's pertinent to one entity and it's not open.
So in my voter I did (it doesn't work, hence I googled for this discussion):

protectedfunctionsupports(string$attribute,mixed$subject):bool    {returnin_array($attribute, ProjectPermission::cases(),true) &&$subjectinstanceof Project;    }

And, returning to the DRY principle, in my controller I could do:

    #[Route('/{slug}', name:'project_view', methods: ['GET'])]publicfunctionview(string$slug,Request$request,ProjectRepository$projectRepository):Response    {$project =$projectRepository->findBySlug($slug);if (!$project || !$this->isGranted(ProjectPermission::View,$project)) {throw$this->createNotFoundException('This project does not exist');        }return$this->render('project/view.html.twig', ['project' =>$project,        ]);    }

I think it's beautiful and clean. However, without enums I need to use constants, then do an array of all the constants for callingin_array in thesupports() method, then use those constants in the controller… Okay, of course, but not as good, in my opinion.
I've been developing Symfony apps for more than five years but I'm quite new at hacking internal Symfony code. so, if there is something against Symfony philosophy here, I'd be glad to understand.
Thank you for reading!

Comment options

What about usingBackedEnum::tryFrom

#[IsGranted(attribute: Permission::SOMETHING->value)]
protectedfunctionsupports(string$attribute,mixed$subject):bool{$user =$this->security->getUser();if (!$userinstanceof UserInterface) {returnfalse;    }return Permission::tryFrom($attribute)instanceof Permission;}
You must be logged in to vote
0 replies
Comment options

I am by no means a reference is anything symfony-related but here are my two cents on this topic nonetheless.

I honestly don't see how allowing a\StringBackedEnum|Expression|string parameter instead of onlyExpression|string on the outer edge security-related attributeIsGranted could be an issue...

I.e, given a Role enum defined as the following :

enum Role:string {    ADMIN = 'ADMIN';    USER = 'USER';}

passingRole::ADMIN vs'ADMIN' to\Symfony\Component\Security\Http\Attribute\IsGranted is not that different...
It looks like the\Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface::isGranted already can support the enums without having to use the backing value anyway so the issue here is realy limited to the IsGranted attribute itself.

However I don't think forcing people to go through hoops by using an imposed string format withRole::ADMIN->value or (even worse looking) using the expression language with something likeenum(PATH\\TO\\NAMESPACE\\ROLE::ADMIN) is a very smart path.

And forcing people to use constants instead of enums would definitely go against the exact argument used earlier : roles are supposed to be a closed list.
Therefore they should be supposed to be represented by enums

Also, simply allowing\StringBackedEnum|Expression|string as a type instead ofExpression|string does not cause any "closed list" issue (as previously mentioned) but allows for a definitely cleaner and safer code (no developer could inadvertendly use an invalid value since all valid values come from the project-defined enum).

All it would take is one line of code to automatically retrieve the value property if the passed element is a StringBackedEnum...
It would not even need to impact the core code as the conversion could happen on an outer edge element, in the IsGrantedAttributeListener class.

You must be logged in to vote
1 reply
@stof
Comment options

stofJun 25, 2025
Collaborator

\StringBackedEnum does not exist in PHP

Comment options

Is this implemented now?
I too cannot understand the arguments against it.
I cannot see how security-http would be coupled in any way by this because from my point of view you would just have something like:

if (is_backed_enum($attribute)) {  $attribute = get_backed_enum_value($attribute);}

inIsGrantedAttributeListener which would be 100% BC.

Coding principles shouldnt undermine the dev experience.
I bet at least 99% of developer has someRoles class with a bunch of constants and their first thought when using symfony after php enums came out was: "Lets use enums for roles".

You must be logged in to vote
10 replies
@mbabker
Comment options

enums enable typing only when you target a specific enum. If the parameter type is \BackedEnum to support any backed enum, you loose all typing benefits.

For clarification. I was referencing typing in domain layer not the framework.

That's kind of the issue though. For your domain to use your types consistently, the framework has to support your domain's type somehow. The framework can't support anApp\Role enum, it can supportBackedEnum orUnitEnum (if you don't want to use backed enums). But even if the framework fully supported enums in the authorization-related code, by the time you get down to your Voter implementation, you still can't rely on your domain types without type assertions or juggling. So even if you could write#[IsGranted(Role::Admin) in your controller, you're still mapping framework types (strings, generic enums) to domain types (App\Role enum) inVoter::supportsAttribute() orVoter::supports().

It's always safer to start final and then open something up versus defaulting to a "everything has to be open" policy, especially with a platform as widely used as Symfony. A fair number of attribute classes started final and were then opened through later PRs for very similar reasons.

Voter would only require simple changes to provide a default implementation.

While this is true in theory, it would also force B/C breaking changes on everyone using theCacheableVoterInterface or the abstractVoter class because they'd all have to support more than string attributes. So that is now a very legitimate concern to keep in mind, how does the impact of that change balance out against the convenience of allowing an application's enum types to flow through the authorization-related interfaces.

@stof
Comment options

stofJun 26, 2025
Collaborator

Making voters generate the cache key does not work: it would require knowing which voter to ask for the cache key before being able to read the cache to know which voter supports that attribute, which is a chicken-and-egg problem.

@Amunak
Comment options

That's kind of the issue though. For your domain to use your types consistently, the framework has to support your domain's type somehow.

No it doesn't; like, ideally it would, but having easy support for it in the application code is much more important.

I.e. having correct, strict typing for methods likeaddRole and whatnot is what's the most desirable here (as well as using it in place of strings in attributes and such); the framework having type-checking for them is very much secondary.

One option to make this way more versatile while also allowing implementation flexibility would be to completely ignore Enums, and instead allow a new Interface to be accepted by the annotation, voters, etc. For example, we could have a RoleInterface, that would have a single method returning string (calledasRoleName or something similar), and then people could use whatever they want: an enum, a regular class, or literally anything.

@stof
Comment options

stofJun 27, 2025
Collaborator

@Amunak you can already use whatever representation you want in your own code (strings, enum, a doctrine relation, etc...) for the roles, as long as you handle converting them to the string type expected bysymfony/security-core when implementingUserInterface::getRoles

@Amunak
Comment options

@stof but that still doesn't allow you to also just use the app's implementation for the security attributes and such. This would be a nice way to unify it all, in my opinion.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Category
Q&A
Labels
None yet
13 participants
@sadikoff@mbabker@stof@tarlepp@Menelion@Amunak@manuakasam@rrajkomar@joshpme@crtl@Justinas-Jurciukonis@DylanKas@FactorSpeed

[8]ページ先頭

©2009-2025 Movatter.jp