Spring Security
  1. Spring Security
  2. SEC-1176

Provide access violated in org.springframework.security.AccessDeniedException

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Won't Fix
    • Affects Version/s: 2.0.4
    • Fix Version/s: 3.0.0 M2
    • Component/s: Core
    • Labels:
      None

      Description

      Provide access violated in org.springframework.security.AccessDeniedException. It would be useful in instances of org.springframework.security.ui.AccessDeniedHandler to know what access was violated in order to redirect the user more clearly.

      String exception.getAccessDenied()

      would be helpful.

        Activity

        Hide
        Luke Taylor added a comment -

        Could you explain the use case more clearly please?

        Show
        Luke Taylor added a comment - Could you explain the use case more clearly please?
        Hide
        Jon Stockdill added a comment -

        In org.springframework.security.ui.AccessDeniedHandler.handle(ServletRequest, ServletResponse, AccessDeniedException), is it possible to know what was the cause of the exception?

        It would be nice to know more details from the exception, so the request can be routed properly. Currently, I am retrieving the user from the context and determining where the request should be routed by the users GrantedAuthority, which creates some messy if else logic. If I knew which GrantedAuthority was missing for a given request it seems like the AccessDeniedHandler would be a little cleaner and less error prone.

        Show
        Jon Stockdill added a comment - In org.springframework.security.ui.AccessDeniedHandler.handle(ServletRequest, ServletResponse, AccessDeniedException), is it possible to know what was the cause of the exception? It would be nice to know more details from the exception, so the request can be routed properly. Currently, I am retrieving the user from the context and determining where the request should be routed by the users GrantedAuthority, which creates some messy if else logic. If I knew which GrantedAuthority was missing for a given request it seems like the AccessDeniedHandler would be a little cleaner and less error prone.
        Hide
        Luke Taylor added a comment -

        AccessDeniedHandler is normally only called when an authenticated user attempts to access something they should not. The normal flow of the application should prevent them from doing this, so it should only happen if they are deliberately manipulating URLs (or something) in order to try to circumvent their normal boundaries or if there is a bug in the application. So I'm still not sure what the scenario is that makes you want to do this. If you could explain that it might be clearer.

        Furthermore, you seem to be assuming that the information would contain a (single?) GrantedAuthority name. The framework is not limited to checking required roles against a list of allowed ones and an AccessDeniedException could occur for many reasons. For example:

        a) The user has an authority which is denied access, or multiple authorities are required and they only have a subset
        b) An ACL denies access to a domain object within the application.
        c) An expression evaluation (in Spring Security 3) denies access
        d) The user has a disallowed IP address
        e) A custom voter denies access (e.g. the time of day is invalid)

        The decision could arise as a combination of votes from a list of registered AccessDecisionVoters, rather than for a single reason.

        So the use of a simple String is not sufficiently generic enough to cater for all the possibilities. In fact, I'd say it's not something that the framework can handle for you. If you really need extra information, then it would be better to implement AccessDecisionManager directly (possibly decorating a standard implementation). You can then wrap the exceptions that or thrown or throw custom ones yourself which match your needs.

        Show
        Luke Taylor added a comment - AccessDeniedHandler is normally only called when an authenticated user attempts to access something they should not. The normal flow of the application should prevent them from doing this, so it should only happen if they are deliberately manipulating URLs (or something) in order to try to circumvent their normal boundaries or if there is a bug in the application. So I'm still not sure what the scenario is that makes you want to do this. If you could explain that it might be clearer. Furthermore, you seem to be assuming that the information would contain a (single?) GrantedAuthority name. The framework is not limited to checking required roles against a list of allowed ones and an AccessDeniedException could occur for many reasons. For example: a) The user has an authority which is denied access, or multiple authorities are required and they only have a subset b) An ACL denies access to a domain object within the application. c) An expression evaluation (in Spring Security 3) denies access d) The user has a disallowed IP address e) A custom voter denies access (e.g. the time of day is invalid) The decision could arise as a combination of votes from a list of registered AccessDecisionVoters, rather than for a single reason. So the use of a simple String is not sufficiently generic enough to cater for all the possibilities. In fact, I'd say it's not something that the framework can handle for you. If you really need extra information, then it would be better to implement AccessDecisionManager directly (possibly decorating a standard implementation). You can then wrap the exceptions that or thrown or throw custom ones yourself which match your needs.
        Hide
        Jon Stockdill added a comment -

        Thanks for your reply.

        Yeah, the AccessDeniedException would need to return something more than String, perhaps it would return differente exceptions depending on the implementation of AccessDecisionManager.

        As for my case, depending on a users GrantedAuthorities, they are redirected to different pages maintain by different hosts. So say if they don't have CREDIT_APPROVED, they are routed to the credit approval page, but if they are not AUTHENTICATED they are routed to the email verification page.

        It does seem like decorating the AccessDecisionManager would provide the desired functionality, possibly implementing my own voters as well. I read this thread:
        http://stackoverflow.com/questions/510846/how-to-throw-an-informative-exception-from-accessdecisionmanager-that-uses-voters

        which also covered the issue. Are there better solutions, none of these seem awesome, but hacks around the issue.

        Fundamentally it seems like AccessDeniedHandler should have access to the votes somehow. Just passing an simple exception seems limiting.

        Show
        Jon Stockdill added a comment - Thanks for your reply. Yeah, the AccessDeniedException would need to return something more than String, perhaps it would return differente exceptions depending on the implementation of AccessDecisionManager. As for my case, depending on a users GrantedAuthorities, they are redirected to different pages maintain by different hosts. So say if they don't have CREDIT_APPROVED, they are routed to the credit approval page, but if they are not AUTHENTICATED they are routed to the email verification page. It does seem like decorating the AccessDecisionManager would provide the desired functionality, possibly implementing my own voters as well. I read this thread: http://stackoverflow.com/questions/510846/how-to-throw-an-informative-exception-from-accessdecisionmanager-that-uses-voters which also covered the issue. Are there better solutions, none of these seem awesome, but hacks around the issue. Fundamentally it seems like AccessDeniedHandler should have access to the votes somehow. Just passing an simple exception seems limiting.
        Hide
        Luke Taylor added a comment -

        It still sounds like you are using this for application control flow, when the decision should be made beforehand what the user can do, what links they can click on, what pages they see etc. As I've said, AccessDeniedHandler is intended to handle a situation when something occurs that generally shouldn't - your application should prevent it from happening in normal usage.

        So your application workflow should send them to the credit approval page as part of the application workflow without relying on a security exception to drive the decision. That would be the better solution in my opinion.

        I've already covered why this doesn't make sense as a general framework feature. When you say "access to the votes somehow" there isn't even any guarantee that the AccessDecisionManager implementation that is being used is voter-based. In fact, as the last comment in that thread says, maybe voters are not the best approach in that situation.

        Show
        Luke Taylor added a comment - It still sounds like you are using this for application control flow, when the decision should be made beforehand what the user can do, what links they can click on, what pages they see etc. As I've said, AccessDeniedHandler is intended to handle a situation when something occurs that generally shouldn't - your application should prevent it from happening in normal usage. So your application workflow should send them to the credit approval page as part of the application workflow without relying on a security exception to drive the decision. That would be the better solution in my opinion. I've already covered why this doesn't make sense as a general framework feature. When you say "access to the votes somehow" there isn't even any guarantee that the AccessDecisionManager implementation that is being used is voter-based. In fact, as the last comment in that thread says, maybe voters are not the best approach in that situation.
        Hide
        Jon Stockdill added a comment -

        Handling access-based routing as part of the application workflow violates DRY and duplicates the access control provided by spring security. Why should the application be required implement the same logic as the security layer? If a user clicks on a page they do not have access to, why shouldn't the security layer support routing the user to an appropriate state?

        AccessDecisionManager decides whether access is granted, but what decides what to do if the access is not granted? AccessDecisionManager cannot control routing since it does not have access to the request/response objects and the AccessDeniedHandler does not have enough information to handle routing, leaving the developer ill equipped to handle route access denied decisions.

        Show
        Jon Stockdill added a comment - Handling access-based routing as part of the application workflow violates DRY and duplicates the access control provided by spring security. Why should the application be required implement the same logic as the security layer? If a user clicks on a page they do not have access to, why shouldn't the security layer support routing the user to an appropriate state? AccessDecisionManager decides whether access is granted, but what decides what to do if the access is not granted? AccessDecisionManager cannot control routing since it does not have access to the request/response objects and the AccessDeniedHandler does not have enough information to handle routing, leaving the developer ill equipped to handle route access denied decisions.
        Hide
        Luke Taylor added a comment -

        Your application shouldn't display links to pages which the user cannot access. You can query the security information to make those decisions in your application, and you can test whether certain operations are possible, but if you try to use the security access-control layer to govern your application flow then you are trying to make it do something it wasn't intended to. It's not webflow or a workflow engine.

        If you are convinced you want to do it this way, then implement your own AccessDecisionManager directly, as suggested above and in the article you pointed to. I don't see any approach to do what you want that makes sense for Spring Security applications in general.

        Show
        Luke Taylor added a comment - Your application shouldn't display links to pages which the user cannot access. You can query the security information to make those decisions in your application, and you can test whether certain operations are possible, but if you try to use the security access-control layer to govern your application flow then you are trying to make it do something it wasn't intended to. It's not webflow or a workflow engine. If you are convinced you want to do it this way, then implement your own AccessDecisionManager directly, as suggested above and in the article you pointed to. I don't see any approach to do what you want that makes sense for Spring Security applications in general.

          People

          • Assignee:
            Luke Taylor
            Reporter:
            Jon Stockdill
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: