[Security] AccessDecisionManager refactoring #942
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 3 commits
May 14, 2011 12:29
…egies into separate AccessStrategyInterface classes.
… previous refactoring.
|
There is already one interface We could still split the strategies in different classes using inheritance instead of composition; something like this:
|
|
That seems a good idea too! |
|
Closing this PR... and waiting for another one based on Johannes idea. |
|
I'm working on it ;) |
nicolas-grekas
added a commit
that referenced
this pull request
Jul 12, 2017
…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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi Fabien, hi Johannes,
I've tried to refactor the AccessDecisionManager class in the security component. I split the three access decision strategies (affirmative, consensus and unanimous) into three dedicated classes. Each strategy class implements the AccessStrategyInterface interface to enforce they have the same public API.
With this changeset, it's now easier to add new access decision strategies into the AccessDecisionManager object.
I couldn't manage to check if any regression came up because I have the following exception:
I'm not sure this exception is due to my changeset. Could you please test this pull request or help me to find and understand why I have this exception.
Thanks.
Hugo.