Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
[Security] Be able to know the reasons of the denied access#40711
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Co-Authored-By: Dany Maillard <danymaillard93b@gmail.com>Co-Authored-By: Antoine Makdessi <antoine.makdessi@agriconomie.com>
This comment has been minimized.
This comment has been minimized.
b56fa30
to93e9c87
Comparesrc/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
some early reviews
@@ -33,7 +33,7 @@ interface VoterInterface | |||
* @param mixed $subject The subject to secure | |||
* @param array $attributes An array of attributes associated with the method being invoked | |||
* | |||
* @return int either ACCESS_GRANTED, ACCESS_ABSTAIN, or ACCESS_DENIED | |||
* @return int|Vote either ACCESS_GRANTED, ACCESS_ABSTAIN, or ACCESS_DENIED |
noniagriconomieApr 6, 2021 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
related to comment onVoter/Voter.php
use AccessTrait; | ||
private $reason; | ||
private $parameters; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
i would call this$context
like in workflow, wdyt?
@@ -206,4 +216,22 @@ private function decidePriority(TokenInterface $token, array $attributes, $objec | |||
return $this->allowIfAllAbstainDecisions; | |||
} | |||
private function decideIfAllAbstainDecisions(): AccessDecision |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
i would directly write this code, the function seem not mandatory
@@ -75,7 +108,7 @@ abstract protected function supports(string $attribute, $subject); | |||
* | |||
* @param mixed $subject | |||
* | |||
* @returnbool | |||
* @returnVote Returning a boolean is deprecated since Symfony 5.1. Return a Vote object instead. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
typo, 5.3
also, shouldnt we create someting like#35592 (comment) to make it BC?
bd948f7
to5489fe2
Compareb5f1e4c
to5b89b7e
Compare👋 ping@azjezz will you be able to rebase and finish for v5.4? thx |
Let's close for now as there is no more work here.@azjezz Feel free to reopen if you'd like to finish it. |
yellow1912 commentedSep 23, 2021
@yellow1912 feel free to do so, I'm currently unable to do so due to lack of time :) |
This is a PR continuing the work started by@maidmaid and@noniagriconomie in#35592
About the voters, the gif of the the original issue sums up the situation well :
Currently, the voters can only say yes or no. This PR allows to give a reason from the voters.
Before :
After:
A voter returns no more an int based on the constant ACCESS_{GRANTED,ABSTAIN,DENIED} but a Vote object which contains the access result and the reason. Thus, the security panel of the profiler can display the reason of each vote.
The access decision manager returns no more a bool for the final verdict but an AccessDecision object which contains the final verdict and all the votes. Thus, the 403 response can display all the denied raisons.
TODO: