Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing denied roles in AccessDeniedException #2380

Closed
wujashek opened this issue Oct 11, 2011 · 8 comments
Closed

Missing denied roles in AccessDeniedException #2380

wujashek opened this issue Oct 11, 2011 · 8 comments

Comments

@wujashek
Copy link

I'm developing heavy javascript application. I need to customize user interaction based on which role voter denied access (show login page, register window or subscription payment). Can AccessDeniedException have roles populated (as an class attribute) by decision manager ? It is very easy solution to implement. Any other better solution ? It will be helpful also for standard application to customize error page by changing the message based on role denied.

@immutef
Copy link
Contributor

immutef commented Mar 16, 2012

Right now its not possible to determine the missing role(s) if access is denied. AccessDecisionManager simply decides whether you have access or not and the interface is not designed to let you know why. You could implement your own AccessDecisionManager which introduces a new method where you can ask a/the voter(s) what is missing to actually gain access and implement a new AccessListener which makes use of this information.

@stof
Copy link
Member

stof commented Apr 4, 2012

@schmittjoh ping

@schmittjoh
Copy link
Contributor

I think this is impossible to implement reliably.

Better do such checks in your access denied handler.

@benjamindulau
Copy link
Contributor

@schmittjoh

IMO this behaviour is really a "must have" and not having it, is, in some way, a pain in the ass ;-)

We're actually developping a project including a subscription system, and we use a specific subscription voter to control either or not an user has reached his plan limits.

But when he has, we can't propose to him to upgrade his subscription to a better plan because we have no way to know "why" the access is denied.

Would it be really problematic to store somehow, in-memory, the different voter decisions, and pass the result to the AccessDeniedHandler ? If we could, at minimum, know which voter denied access, we could easily identify why.

@armetiz
Copy link
Contributor

armetiz commented Aug 17, 2012

I agree with @benjamindulau and @wujashek.
AccessDecisionManager::decide shouldn't return Boolean but a more specific object.

But, It Seems that implies many BC..

@borisguery
Copy link

I absolutely agree with you, the feature is needed as far as you need a fine grained authorization system. As @benjamindulau said, given a 3 levels subscription system with monthly fees, how could I determine why my user is not granted to access a resource. Does his subscription level too low (trying to access a silver resource while being bronze)? Does my user has paid his monthly fee? Does my user has been temporarily suspended? There are many reasons an user can't be granted access to a resource, and we need to know why.

@andreasschmidt
Copy link

Any updates concerning this issue? It is really hard to write clean code without knowing the reason why the user has no access to the resource:

if (!$this->security->isGranted("ROLE_USERNAME")) {
$path = 'common_registration_enter_username';
} else if (!$this->security->isGranted("ROLE_EMAIL")) {
$path = 'common_registration_confirm_email';
} else if (!$this->security->isGranted("ROLE_PROFILE")) {
$path = 'common_registration_complete_profile';
} else if (!$this->security->isGranted("ROLE_VERIFIED")) {
$path = 'common_security_verification_needed';
}

@benjamindulau
Copy link
Contributor

@andreasschmidt As a temporary work around you can use some kind of VoterDecisionCollector, that's the cleanest solution i found.

My voters must implement a NamedVoterInterface (extending the default VoterInterface) which only defines a getName() method.

After that, every voter is injected with the VoterDecisionCollector and must add each of its decision to it. This way I can access these decisions anywhere in the App.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants