Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Security] Expose the required roles in AccessDeniedException#19473
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
HeahDude commentedJul 29, 2016
Tests are failing Status: Needs Work |
HeahDude commentedJul 29, 2016
👍 Thanks Tristan! Status: Reviewed |
fabpot commentedJul 29, 2016
👍 |
stof commentedJul 29, 2016
👍 Would be great to open a PR on SensioFrameworkExtraBundle making use of these setters when they are available, for its checks on the |
| throw$this->createAccessDeniedException($message); | ||
| $exception =$this->createAccessDeniedException($message); | ||
| $exception->setAttributes($attributes); | ||
| $exception->setObject($object); |
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.
The object doesn't have to be an object as far as I know. Since a recent version scalars are also supported I believe. Not sure if setObject is still correct in that case.
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.
object is the name already used by the security component
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.
The VoterInterface use subjet instead of object.
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.
Indeed, I would call itSubject
nicolas-grekas commentedAug 9, 2016
👍 |
fabpot commentedAug 9, 2016
Thank you@Nicofuma. |
…ception (Nicofuma)This PR was merged into the 3.2-dev branch.Discussion----------[Security] Expose the required roles in AccessDeniedException| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | yes| BC breaks? | no| Deprecations? | no| Tests pass? | yes| License | MITNowadays it is more and more common to protect some sensitive actions and part of a website using 2FA or some re-authentication mechanism (per example, on Github you have to enter your password again when you add an ssh key). But currently, in Symfony, it is really hard to implement without having to duplicate the logic, provide an explicit list of URLs to protect or hack into the security component.A good way to achieve that would be to add a special role (like IS_AUTHENTICATED_FULLY) and use it in the access map. But it requires us to be able to have a custom logic in an ExceptionListener depending on the roles behind an AccessDeniedException.With this patch we could write an ExceptionListener of this kind (a similar logic could also be used in an AccessDeniedHandler):```php public function onKernelException(GetResponseForExceptionEvent $event) { $exception = $event->getException(); do { if ($exception instanceof AccessDeniedException) { foreach ($exception->getAttributes() as $role) { if ($role === 'IS_AUTHENTICATED_2FA' && !$this->accessDecisionManager->decide($this->tokenStorage->getToken(), $role, $exception->getObject())) { // Start 2FA } } } } while (null !== $exception = $exception->getPrevious()); }```Replaces#18661Commits-------6618c18 [Security] Expose the required roles in AccessDeniedException…ct (xabbuh)This PR was merged into the 3.2-dev branch.Discussion----------[Security] AccessDeniedException: rename object to subject| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#19473 (comment)| License | MIT| Doc PR |With this change the name is inline with what we use in the base voterinterface.Commits-------9603ffa AccessDeniedException: rename object to subject
Uh oh!
There was an error while loading.Please reload this page.
Nowadays it is more and more common to protect some sensitive actions and part of a website using 2FA or some re-authentication mechanism (per example, on Github you have to enter your password again when you add an ssh key). But currently, in Symfony, it is really hard to implement without having to duplicate the logic, provide an explicit list of URLs to protect or hack into the security component.
A good way to achieve that would be to add a special role (like IS_AUTHENTICATED_FULLY) and use it in the access map. But it requires us to be able to have a custom logic in an ExceptionListener depending on the roles behind an AccessDeniedException.
With this patch we could write an ExceptionListener of this kind (a similar logic could also be used in an AccessDeniedHandler):
Replaces#18661