SEC-884: The Authorization Tag Libraries should use the AccessDecisionManager #1136

Closed
spring-issuemaster opened this Issue Jun 16, 2008 · 9 comments

1 participant

@spring-issuemaster

Thomas Champagne (Migrated from SEC-884) said:

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

@spring-issuemaster

Thomas Champagne said:

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.

@spring-issuemaster

Luke Taylor said:

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.

@spring-issuemaster

Willie Wheeler said:

I assume this is why IS_AUTHENTICATED_ANONYMOUSLY, IS_AUTHENTICATED_REMEMBERED and IS_AUTHENTICATED_FULLY don’t currently work in the tag…

@spring-issuemaster

Pavel Tcholakov said:

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

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.

@spring-issuemaster

Luke Taylor said:

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

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

@spring-issuemaster

Luke Taylor said:

@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.

@spring-issuemaster

Pavel Tcholakov said:

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.

@spring-issuemaster

Luke Taylor said:

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

@spring-issuemaster

Knut Vidar Siem said:

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?

@spring-issuemaster spring-issuemaster added this to the 3.0.0 RC1 milestone Feb 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment