3.0.7.RELEASE and RoleHierarchyImpl with @PreAuthorize framework bug ?

[dlmiles]

I am not able to make @PreAuthorize("hasRole('ROLE_ADMIN')") work as I expect, in that it will permit anyone with ROLE_ADMIN or ROLE_DEVEL access to the annotated method.

Authentication works correctly, I believe I am putting excessive GrantedAuthority's into the returned Set, i.e. with RoleHierarchyImpl in use I should only need to put the role a single role into the set relating to that users security clearance. I did debug Spring framework code to see that the correct list of extrapolated GrantedAuthority's is calculated.

The @PreAuthorize annotation over a method is being intercepted and redirected to the Access Denied page.


I tracked the probem down to org.springframework.security.access.vote.RoleVoter #supports(ConfigAttribute attribute) always returning false, because attribute.getAttribute() always return null. Now you will notice from my config I only instate RoleHierarchyVoter and that the implementation extends RoleVoter, so I presume this is how execution gets to this point.

The code in RoleVoter makes to call to #supports(ConfigAttribute) and because that always returns false it can never return anything other than ACCESS_ABSTAIN.

Is this a framework bug or am I missing some additional bug necessary configuration ?

// THIS IS FROM SPRING FRAMEWORK CODE RoleVoter.java
   public int vote(Authentication authentication, Object object, Collection<ConfigAttribute> attributes) {
        int result = ACCESS_ABSTAIN;
        Collection<GrantedAuthority> authorities = extractAuthorities(authentication);

        for (ConfigAttribute attribute : attributes) {
            if (this.supports(attribute)) {    // this is how supports() gets called, but it always returns false here
                result = ACCESS_DENIED;

                // Attempt to find a matching granted authority
                for (GrantedAuthority authority : authorities) {
                    if (attribute.getAttribute().equals(authority.getAuthority())) {
                        return ACCESS_GRANTED;
                    }
                }
            }
        }

        return result;
    }

// THIS IS MY CONFIG
<global-method-security pre-post-annotations="enabled" secured-annotations="enabled" access-decision-manager-ref="accessDecisionManager">
	</global-method-security>

	<http use-expressions="true">
		<intercept-url pattern="/login*" access="permitAll"/>
		<intercept-url pattern="/logout*" access="permitAll"/>
		<intercept-url pattern="/resources/**" access="permitAll"/>
		<intercept-url pattern="/users" access="hasRole('ROLE_ADMIN')"/>
		<intercept-url pattern="/users/**" access="hasRole('ROLE_ADMIN')"/>
		<intercept-url pattern="/**" access="isAuthenticated()"/>
		<form-login login-processing-url="/resources/j_spring_security_check" login-page="/login" authentication-failure-url="/login?login_error=t" />
		<logout logout-url="/resources/j_spring_security_logout" />
		<session-management session-fixation-protection="newSession"/>
		<remember-me/>
	</http>

	<beans:bean id="roleHierarchy" class="org.springframework.security.access.hierarchicalroles.RoleHierarchyImpl">
		<beans:property name="hierarchy">
			<beans:value>
				ROLE_DEVEL > ROLE_ADMIN
				ROLE_ADMIN > ROLE_USER
				ROLE_USER > ROLE_VISITOR
			</beans:value>
		</beans:property>
	</beans:bean>

	<beans:bean id="roleHierarchyVoter" class="org.springframework.security.access.vote.RoleHierarchyVoter">
		<beans:constructor-arg ref="roleHierarchy" />
	</beans:bean>

	<beans:bean id="accessDecisionManager" class="org.springframework.security.access.vote.AffirmativeBased">
		<beans:property name="decisionVoters">
			<beans:list>
				<beans:ref bean="roleHierarchyVoter"/>
			</beans:list>
		</beans:property>
	</beans:bean>

My custom org.springframework.security.core.userdetails.User Details has code like (ignore the comments thety related to things I've tried to get it working based on information I read; ideally the "break;" statements can be reinstated but right now when it is working) :

// THIS IS MY RE-IMPLEMENTATION OF UserDetails (THIS CODE WORKS JUST AS EXPECTED BUT HERE TO SHOW SETUP OF GrantAuthority)
public class WebappUserDetails implements UserDetails, CredentialsContainer {
	/**
	 * 
	 */
	private static final long serialVersionUID = 1L;

	private long id;
	private long version;
	private String username;
	private String password;
	private char state;
	private List<GrantedAuthority> authorities;

	private WebappUserDetails() {
		authorities = new ArrayList<GrantedAuthority>();
	}

	public static WebappUserDetails createFromAccountCredential(AccountCredential accountCredential) {
		if(accountCredential.getState() == AccountCredential.STATE_DELETED)
			return null;	// WebappUserDetails we don't allow creation of DELETED data

		WebappUserDetails webappUserDetails = new WebappUserDetails();
		webappUserDetails.id = accountCredential.getAccountId();
		webappUserDetails.version = accountCredential.getVersion();
		webappUserDetails.username = accountCredential.getUsername();
		webappUserDetails.password = accountCredential.getPassword();
		webappUserDetails.state = accountCredential.getState();

		SortedSet<String> grantedAuthoritySet = new TreeSet<String>();;
		// need to add all the roles a user is a member of ? did not expect this with RoleHierarchyVoter in use
		switch(accountCredential.getAccountType()) {
		case AccountCredential.AT_DEVEL:
			grantedAuthoritySet.add("ROLE_DEVEL");
			//break;
		case AccountCredential.AT_ADMIN:
			grantedAuthoritySet.add("ROLE_ADMIN");
			//break;
		case AccountCredential.AT_USER:
			grantedAuthoritySet.add("ROLE_USER");
			break;
		}

		// Hmm we do it like this as it needs to be sorted: // FIXME maybe this is not correct statement
		for(String role : grantedAuthoritySet) {
			GrantedAuthority grantedAuthority = new GrantedAuthorityImpl(role);
			webappUserDetails.authorities.add(grantedAuthority);
		}

		return webappUserDetails;
	}

	@Override
	public Collection<GrantedAuthority> getAuthorities() {
		return Collections.unmodifiableCollection(authorities);
	}

	/// other getters/interface API omitted from this snippet
	
}

[dlmiles]

No ideas on this one ?

IIRC the original code was that generated by Spring ROO and part of the spring-security git example so taken to be somewhat correct.

[Rob Winch]

Put together a working example of your problem and make it available (i.e. put it on github). The easier it is for me to try the faster I will be able to respond.