Missing access decision strategy "highest not abstained voter" #14049
Comments
|
Is there any chance such a thing would be accepted if I write a PR for it? And if yes, how should this strategy be called? |
|
It's not up to me, but i would actually not implement another strategy. Instead maybe an option is to see if we could get rid of the hard coded strategies all together and have them pluggable just like pretty much everything else in the system. It shouldn't be very difficult I think, but it would make it a lot easier add custom strategies if needed (as in your case), as it might be something as easy as implementing an interface and tagging with something like |
|
Totally true, but that sounds like a BC break, doesn't it? |
|
It could certainly be done without a BC break. @danrot I think your setup is pretty typical - where you have multiple voters, but only 1 voter ever votes at any time. So, the count is always 1 for access granted, and everyone else abstains. Is this accurate? So in this case, I keep the default strategy - it works fine. I think the only problem would be if I screwed up, and 2 voters voted, but that would also be a problem with the new strategy - the first would be used, but I may have intended for the second to be called (but I messed something up). Or perhaps I'm completely misunderstanding your situation. Let me know. I'm not convinced we have enough of a problem to need a new strategy. |
|
@weaverryan The problem was that there was the possibility that two voter could vote, but one has a higher priority. So if the higher prioritized voter has an opinion, take this value, if not, try the next voter in the list. Sadly I can't remember the exact use case, and in a fast search I also didn't find the location in the code anymore... But if I remember correctly I have solved this issue with introducing a |
|
Right after I pressed comment I realized the exact problem again 😄 We have a security context, which describes an area in the application (e.g. this user is allowed to enter the content area). Additionally we are using an ACL for single pages in the content area, which overrides the previous right. So even if a user is missing the right to enter the content area, he can edit a single page, if this page allows him to. So the idea was to check the acl first, and take this value if it allows or denies the operation. Only if it abstains the second voter for the content area would be called, kind as a fallback. Do you understand what I mean? |
|
@danrot hmm, but there are no priorities in voters, right? The fact that one is first is sort of "accidental" irrc. In you case - just playing devil's advocate - the strategy that allows access if any voter says "Granted" would also work I think. |
|
@weaverryan No, it would not, because if the acl denies access, but the user has access to the general area, I would like the result to be denied. But you are right, looks like I've confused something... There are really no priorities on voters, but IMO this concept would still be interesting. |
HighestNotAbstainedVoterStrategy has been added. Also this commit makes access decision strategy extensible.
HighestNotAbstainedVoterStrategy has been added. Also this commit makes access decision strategy extensible.
HighestNotAbstainedVoterStrategy has been added. Also this commit makes access decision strategy extensible.
HighestNotAbstainedVoterStrategy has been added. Also this commit makes access decision strategy extensible.
HighestNotAbstainedVoterStrategy has been added. Also this commit makes access decision strategy extensible.
|
@danrot we have priority on voters (the component does not as it expects to register the voters in the order in which they will be called) |
HighestNotAbstainedVoterStrategy has been added. Also this commit makes access decision strategy extensible.
|
I could also really use this feature. Another Use case would be to create an |
|
I think making the access decision strategy injectable is what we should do here. We recently rejected another PR that tried to implement another strategy which would then be possible in custom code too. |
|
I finally found some time to work on the approach I described in the previous comment. See #19034 for the pull request. |
…ss decision manager service (xabbuh) This PR was merged into the 3.4 branch. Discussion ---------- [Security] make it possible to configure a custom access decision manager service | Q | A | | --- | --- | | Branch? | 3.4 | | Bug fix? | no | | New feature? | yes | | BC breaks? | no | | Deprecations? | no | | Tests pass? | yes | | Fixed tickets | #942, #14049, #15295, #16828, #16843, | | License | MIT | | Doc PR | TODO | These changes will make it possible to let users define their own voting strategies without the need for custom compiler passes that replace the built-in `AccessDecisionManager` (see linked issues in the PR table for some use cases). Commits ------- e0913a2 add option to define the access decision manager
I am currently implementing a security system, and I am missing one strategy for the
AccessDecisionManager.I have two voters, but only one of these two should actually vote. My current Workaround would be to put this logic into the voters, but that's IMHO not the best place for it. I would prefer this to be handled by the
AccessDecisionManager. Therefore I would suggest to add another strategy, so that the result of the first voter being not abstain, whether it grants or denies access, should be returned.The text was updated successfully, but these errors were encountered: