Spring Security
  1. Spring Security
  2. SEC-884

The Authorization Tag Libraries should use the AccessDecisionManager

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Won't Fix
    • Affects Version/s: 2.0.2
    • Fix Version/s: 3.0.0 RC1
    • Component/s: Taglibs
    • Labels:
      None

      Description

      The current implementation of AuthorizeTag checks the GrantedAuthorities in the UserDetails.
      If you use a custom voter (RoleHierarchyVoter for example) to check authorithies, the taglib doesn't work.

      The related thread in forum : http://forum.springframework.org/showthread.php?t=55800

        Issue Links

          Activity

          Hide
          Thomas Champagne added a comment -

          This tag is very similar to the AccessCheckerTag in the SEC-525 issue and the parameters are the same as the currently AuthorizeTag : ifAllGranted, ifAnyGranted, ifNotGranted.

          Show
          Thomas Champagne added a comment - This tag is very similar to the AccessCheckerTag in the SEC-525 issue and the parameters are the same as the currently AuthorizeTag : ifAllGranted, ifAnyGranted, ifNotGranted.
          Hide
          Luke Taylor added a comment -

          The taglibs have been around since long before role hierarchies etc. and only offer pretty basic functionality. Any changes here will have to wait till the next major version as they could break existing apps.

          Show
          Luke Taylor added a comment - The taglibs have been around since long before role hierarchies etc. and only offer pretty basic functionality. Any changes here will have to wait till the next major version as they could break existing apps.
          Hide
          Willie Wheeler added a comment -

          I assume this is why IS_AUTHENTICATED_ANONYMOUSLY, IS_AUTHENTICATED_REMEMBERED and IS_AUTHENTICATED_FULLY don't currently work in the <security:authorize> tag...

          Show
          Willie Wheeler added a comment - I assume this is why IS_AUTHENTICATED_ANONYMOUSLY, IS_AUTHENTICATED_REMEMBERED and IS_AUTHENTICATED_FULLY don't currently work in the <security:authorize> tag...
          Hide
          Pavel Tcholakov added a comment -

          I just wanted to note that the attached AuthorizeCheckerTag.java implementation always returns "false" if the authentication is null, so e.g. the following does not work as expected:

          <authz:authorize ifNotGranted="IS_AUTHENTICATED_FULLY">...</authz:authorize>

          The current RoleVoter and RoleHierarchyVoters implementations throw a NPE if called with a null authentication so there is no trivial workaround for the time being. I will log another issue to fix those two, then at least calling accessDecisionManager.decide with a null authentication should not be a problem anymore.

          Show
          Pavel Tcholakov added a comment - I just wanted to note that the attached AuthorizeCheckerTag.java implementation always returns "false" if the authentication is null, so e.g. the following does not work as expected: <authz:authorize ifNotGranted="IS_AUTHENTICATED_FULLY">...</authz:authorize> The current RoleVoter and RoleHierarchyVoters implementations throw a NPE if called with a null authentication so there is no trivial workaround for the time being. I will log another issue to fix those two, then at least calling accessDecisionManager.decide with a null authentication should not be a problem anymore.
          Hide
          Luke Taylor added a comment -

          The existing tags should probably be replaced with a more generic approach which uses the AccessDecisionManager directly. It shouldn't use "ifAllGranted" etc, since these are limited concepts which assume that the attributes are only to be checked against the user's authorities. It would be better to have something like

          <authorize access="expression"/>

          Where the expression could be either a standard list of attributes or potentially even an EL exression.

          Show
          Luke Taylor added a comment - The existing tags should probably be replaced with a more generic approach which uses the AccessDecisionManager directly. It shouldn't use "ifAllGranted" etc, since these are limited concepts which assume that the attributes are only to be checked against the user's authorities. It would be better to have something like <authorize access="expression"/> Where the expression could be either a standard list of attributes or potentially even an EL exression.
          Hide
          Luke Taylor added a comment -

          @Pavel. I'm not quit sure where the issue with NPEs in the voters comes in here, but I think it would be preferable to make the AccessDecisionManager contract specify a non-null Authentication object.

          Show
          Luke Taylor added a comment - @Pavel. I'm not quit sure where the issue with NPEs in the voters comes in here, but I think it would be preferable to make the AccessDecisionManager contract specify a non-null Authentication object.
          Hide
          Pavel Tcholakov added a comment -

          I don't have the code in front of me, but it was quite easy to fix - as you say, just tightening the contract would be fine. Otherwise just define what happens f called with a null authentication.

          Regarding expressions, we wrote a custom Voter which evaluates JEXL expressions and then uses the modified AuthorizeCheckerTag.java above to check them from JSPs. The advantage is that your security logic lives in named scriptlets external to your JSPs and you can reuse them with the HTTP URL interceptor or the method-level security interceptor. We also provide for implementing these in Java, if one so wishes. The whole lot is configured in a Spring Beans XML file using a custom FactoryBean.

          Show
          Pavel Tcholakov added a comment - I don't have the code in front of me, but it was quite easy to fix - as you say, just tightening the contract would be fine. Otherwise just define what happens f called with a null authentication. Regarding expressions, we wrote a custom Voter which evaluates JEXL expressions and then uses the modified AuthorizeCheckerTag.java above to check them from JSPs. The advantage is that your security logic lives in named scriptlets external to your JSPs and you can reuse them with the HTTP URL interceptor or the method-level security interceptor. We also provide for implementing these in Java, if one so wishes. The whole lot is configured in a Spring Beans XML file using a custom FactoryBean.
          Hide
          Luke Taylor added a comment -

          The ability to use expressions with the authorize tag, and to inject a RoleHierarchy into the expression handler should provide the functionality required here.

          Show
          Luke Taylor added a comment - The ability to use expressions with the authorize tag, and to inject a RoleHierarchy into the expression handler should provide the functionality required here.
          Hide
          Knut Vidar Siem added a comment -

          The name authorize/AuthorizeTag implies that the tag deals with authorization, but currently only half-way, as it does not interact with an AccessDecisionManager. Wouldn't it be better if the interaction with the AccessDecisionManager was built into the tag, rather than bolting it on via an expression feature?

          Show
          Knut Vidar Siem added a comment - The name authorize/AuthorizeTag implies that the tag deals with authorization, but currently only half-way, as it does not interact with an AccessDecisionManager. Wouldn't it be better if the interaction with the AccessDecisionManager was built into the tag, rather than bolting it on via an expression feature?

            People

            • Assignee:
              Luke Taylor
              Reporter:
              Thomas Champagne
            • Votes:
              4 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: