Make RoleHierarchyImpl internals a bit simpler.

Issue: gh-7035
This commit is contained in:
Pavel Horal 2019-07-12 18:42:44 +02:00
parent d3eaef66fc
commit be0ad673c2
1 changed files with 93 additions and 105 deletions

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2016 the original author or authors. * Copyright 2002-2019 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -15,9 +15,6 @@
*/ */
package org.springframework.security.access.hierarchicalroles; package org.springframework.security.access.hierarchicalroles;
import java.io.BufferedReader;
import java.io.IOException;
import java.io.StringReader;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collection; import java.util.Collection;
import java.util.HashMap; import java.util.HashMap;
@ -34,41 +31,43 @@ import org.springframework.security.core.authority.SimpleGrantedAuthority;
/** /**
* <p> * <p>
* This class defines a role hierarchy for use with the UserDetailsServiceWrapper. * This class defines a role hierarchy for use with various access checking components.
* *
* <p> * <p>
* Here is an example configuration of a role hierarchy (hint: read the "&gt;" sign as * Here is an example configuration of a role hierarchy (hint: read the "&gt;" sign as "includes"):
* "includes"):
* *
* <pre> * <pre>
* &lt;property name="hierarchy"&gt; * &lt;property name="hierarchy"&gt;
* &lt;value&gt; * &lt;value&gt;
* ROLE_A &gt; ROLE_B * ROLE_A &gt; ROLE_B
* ROLE_B &gt; ROLE_AUTHENTICATED * ROLE_B &gt; ROLE_AUTHENTICATED
* ROLE_AUTHENTICATED &gt; ROLE_UNAUTHENTICATED * ROLE_AUTHENTICATED &gt; ROLE_UNAUTHENTICATED
* &lt;/value&gt; * &lt;/value&gt;
* &lt;/property&gt; * &lt;/property&gt;
* </pre> * </pre>
* *
* <p> * <p>
* Explanation of the above:<br> * Explanation of the above:
* In effect every user with ROLE_A also has ROLE_B, ROLE_AUTHENTICATED and * <ul>
* ROLE_UNAUTHENTICATED;<br> * <li>In effect every user with ROLE_A also has ROLE_B, ROLE_AUTHENTICATED and ROLE_UNAUTHENTICATED;</li>
* every user with ROLE_B also has ROLE_AUTHENTICATED and ROLE_UNAUTHENTICATED;<br> * <li>every user with ROLE_B also has ROLE_AUTHENTICATED and ROLE_UNAUTHENTICATED;</li>
* every user with ROLE_AUTHENTICATED also has ROLE_UNAUTHENTICATED. * <li>every user with ROLE_AUTHENTICATED also has ROLE_UNAUTHENTICATED.</li>
* </ul>
* *
* <p> * <p>
* Hierarchical Roles will dramatically shorten your access rules (and also make the * Hierarchical Roles will dramatically shorten your access rules (and also make the access rules
* access rules much more elegant). * much more elegant).
* *
* <p> * <p>
* Consider this access rule for Spring Security's RoleVoter (background: every user that * Consider this access rule for Spring Security's RoleVoter (background: every user that is
* is authenticated should be able to log out):<br> * authenticated should be able to log out):
* /logout.html=ROLE_A,ROLE_B,ROLE_AUTHENTICATED<br> * <pre>/logout.html=ROLE_A,ROLE_B,ROLE_AUTHENTICATED</pre>
* With hierarchical roles this can now be shortened to:<br> *
* /logout.html=ROLE_AUTHENTICATED<br> * With hierarchical roles this can now be shortened to:
* In addition to shorter rules this will also make your access rules more readable and * <pre>/logout.html=ROLE_AUTHENTICATED</pre>
* your intentions clearer. *
* In addition to shorter rules this will also make your access rules more readable and your
* intentions clearer.
* *
* @author Michael Mayr * @author Michael Mayr
*/ */
@ -76,19 +75,24 @@ public class RoleHierarchyImpl implements RoleHierarchy {
private static final Log logger = LogFactory.getLog(RoleHierarchyImpl.class); private static final Log logger = LogFactory.getLog(RoleHierarchyImpl.class);
/**
* Raw hierarchy configuration where each line represents single or multiple level role chain.
*/
private String roleHierarchyStringRepresentation = null; private String roleHierarchyStringRepresentation = null;
/** /**
* rolesReachableInOneStepMap is a Map that under the key of a specific role name * {@code rolesReachableInOneStepMap} is a Map that under the key of a specific role name
* contains a set of all roles reachable from this role in 1 step * contains a set of all roles reachable from this role in 1 step
* (i.e. parsed {@link #roleHierarchyStringRepresentation} grouped by the higher role)
*/ */
private Map<GrantedAuthority, Set<GrantedAuthority>> rolesReachableInOneStepMap = null; private Map<String, Set<GrantedAuthority>> rolesReachableInOneStepMap = null;
/** /**
* rolesReachableInOneOrMoreStepsMap is a Map that under the key of a specific role * {@code rolesReachableInOneOrMoreStepsMap} is a Map that under the key of a specific role
* name contains a set of all roles reachable from this role in 1 or more steps * name contains a set of all roles reachable from this role in 1 or more steps
* (i.e. fully resolved hierarchy from {@link #rolesReachableInOneStepMap})
*/ */
private Map<GrantedAuthority, Set<GrantedAuthority>> rolesReachableInOneOrMoreStepsMap = null; private Map<String, Set<GrantedAuthority>> rolesReachableInOneOrMoreStepsMap = null;
/** /**
* Set the role hierarchy and pre-calculate for every role the set of all reachable * Set the role hierarchy and pre-calculate for every role the set of all reachable
@ -102,13 +106,16 @@ public class RoleHierarchyImpl implements RoleHierarchy {
public void setHierarchy(String roleHierarchyStringRepresentation) { public void setHierarchy(String roleHierarchyStringRepresentation) {
this.roleHierarchyStringRepresentation = roleHierarchyStringRepresentation; this.roleHierarchyStringRepresentation = roleHierarchyStringRepresentation;
logger.debug("setHierarchy() - The following role hierarchy was set: " if (logger.isDebugEnabled()) {
+ roleHierarchyStringRepresentation); logger.debug("setHierarchy() - The following role hierarchy was set: "
+ roleHierarchyStringRepresentation);
}
buildRolesReachableInOneStepMap(); buildRolesReachableInOneStepMap();
buildRolesReachableInOneOrMoreStepsMap(); buildRolesReachableInOneOrMoreStepsMap();
} }
@Override
public Collection<GrantedAuthority> getReachableGrantedAuthorities( public Collection<GrantedAuthority> getReachableGrantedAuthorities(
Collection<? extends GrantedAuthority> authorities) { Collection<? extends GrantedAuthority> authorities) {
if (authorities == null || authorities.isEmpty()) { if (authorities == null || authorities.isEmpty()) {
@ -116,13 +123,29 @@ public class RoleHierarchyImpl implements RoleHierarchy {
} }
Set<GrantedAuthority> reachableRoles = new HashSet<>(); Set<GrantedAuthority> reachableRoles = new HashSet<>();
Set<String> processedNames = new HashSet<>();
for (GrantedAuthority authority : authorities) { for (GrantedAuthority authority : authorities) {
addReachableRoles(reachableRoles, authority); // Do not process authorities without string representation
Set<GrantedAuthority> additionalReachableRoles = getRolesReachableInOneOrMoreSteps( if (authority.getAuthority() == null) {
authority); reachableRoles.add(authority);
if (additionalReachableRoles != null) { continue;
reachableRoles.addAll(additionalReachableRoles); }
// Do not process already processed roles
if (!processedNames.add(authority.getAuthority())) {
continue;
}
// Add original authority
reachableRoles.add(authority);
// Add roles reachable in one or more steps
Set<GrantedAuthority> lowerRoles = this.rolesReachableInOneOrMoreStepsMap.get(authority.getAuthority());
if (lowerRoles == null) {
continue; // No hierarchy for the role
}
for (GrantedAuthority role : lowerRoles) {
if (processedNames.add(role.getAuthority())) {
reachableRoles.add(role);
}
} }
} }
@ -132,75 +155,40 @@ public class RoleHierarchyImpl implements RoleHierarchy {
+ " in zero or more steps."); + " in zero or more steps.");
} }
List<GrantedAuthority> reachableRoleList = new ArrayList<>( List<GrantedAuthority> reachableRoleList = new ArrayList<>(reachableRoles.size());
reachableRoles.size());
reachableRoleList.addAll(reachableRoles); reachableRoleList.addAll(reachableRoles);
return reachableRoleList; return reachableRoleList;
} }
// SEC-863
private void addReachableRoles(Set<GrantedAuthority> reachableRoles,
GrantedAuthority authority) {
for (GrantedAuthority testAuthority : reachableRoles) {
String testKey = testAuthority.getAuthority();
if ((testKey != null) && (testKey.equals(authority.getAuthority()))) {
return;
}
}
reachableRoles.add(authority);
}
// SEC-863
private Set<GrantedAuthority> getRolesReachableInOneOrMoreSteps(
GrantedAuthority authority) {
if (authority.getAuthority() == null) {
return null;
}
for (GrantedAuthority testAuthority : this.rolesReachableInOneOrMoreStepsMap
.keySet()) {
String testKey = testAuthority.getAuthority();
if ((testKey != null) && (testKey.equals(authority.getAuthority()))) {
return this.rolesReachableInOneOrMoreStepsMap.get(testAuthority);
}
}
return null;
}
/** /**
* Parse input and build the map for the roles reachable in one step: the higher role * Parse input and build the map for the roles reachable in one step: the higher role
* will become a key that references a set of the reachable lower roles. * will become a key that references a set of the reachable lower roles.
*/ */
private void buildRolesReachableInOneStepMap() { private void buildRolesReachableInOneStepMap() {
this.rolesReachableInOneStepMap = new HashMap<>(); this.rolesReachableInOneStepMap = new HashMap<>();
try (BufferedReader bufferedReader = new BufferedReader( for (String line : this.roleHierarchyStringRepresentation.split("\n")) {
new StringReader(this.roleHierarchyStringRepresentation))) { // Split on > and trim excessive whitespace
for (String readLine; (readLine = bufferedReader.readLine()) != null;) { String[] roles = line.trim().split("\\s+>\\s+");
String[] roles = readLine.split(" > ");
for (int i = 1; i < roles.length; i++) { for (int i = 1; i < roles.length; i++) {
GrantedAuthority higherRole = new SimpleGrantedAuthority( String higherRole = roles[i - 1];
roles[i - 1].replaceAll("^\\s+|\\s+$", "")); GrantedAuthority lowerRole = new SimpleGrantedAuthority(roles[i]);
GrantedAuthority lowerRole = new SimpleGrantedAuthority(roles[i].replaceAll("^\\s+|\\s+$", ""));
Set<GrantedAuthority> rolesReachableInOneStepSet; Set<GrantedAuthority> rolesReachableInOneStepSet;
if (!this.rolesReachableInOneStepMap.containsKey(higherRole)) { if (!this.rolesReachableInOneStepMap.containsKey(higherRole)) {
rolesReachableInOneStepSet = new HashSet<>(); rolesReachableInOneStepSet = new HashSet<>();
this.rolesReachableInOneStepMap.put(higherRole, rolesReachableInOneStepSet); this.rolesReachableInOneStepMap.put(higherRole, rolesReachableInOneStepSet);
} else { } else {
rolesReachableInOneStepSet = this.rolesReachableInOneStepMap.get(higherRole); rolesReachableInOneStepSet = this.rolesReachableInOneStepMap.get(higherRole);
} }
rolesReachableInOneStepSet.add(lowerRole); rolesReachableInOneStepSet.add(lowerRole);
if (logger.isDebugEnabled()) {
logger.debug("buildRolesReachableInOneStepMap() - From role " + higherRole if (logger.isDebugEnabled()) {
+ " one can reach role " + lowerRole + " in one step."); logger.debug("buildRolesReachableInOneStepMap() - From role " + higherRole
} + " one can reach role " + lowerRole + " in one step.");
} }
} }
} catch (IOException e) {
throw new IllegalStateException(e);
} }
} }
@ -213,25 +201,25 @@ public class RoleHierarchyImpl implements RoleHierarchy {
private void buildRolesReachableInOneOrMoreStepsMap() { private void buildRolesReachableInOneOrMoreStepsMap() {
this.rolesReachableInOneOrMoreStepsMap = new HashMap<>(); this.rolesReachableInOneOrMoreStepsMap = new HashMap<>();
// iterate over all higher roles from rolesReachableInOneStepMap // iterate over all higher roles from rolesReachableInOneStepMap
for (String roleName : this.rolesReachableInOneStepMap.keySet()) {
for (GrantedAuthority role : this.rolesReachableInOneStepMap.keySet()) { Set<GrantedAuthority> rolesToVisitSet = new HashSet<>(this.rolesReachableInOneStepMap.get(roleName));
Set<GrantedAuthority> rolesToVisitSet = new HashSet<>(this.rolesReachableInOneStepMap.get(role));
Set<GrantedAuthority> visitedRolesSet = new HashSet<>(); Set<GrantedAuthority> visitedRolesSet = new HashSet<>();
while (!rolesToVisitSet.isEmpty()) { while (!rolesToVisitSet.isEmpty()) {
// take a role from the rolesToVisit set // take a role from the rolesToVisit set
GrantedAuthority aRole = rolesToVisitSet.iterator().next(); GrantedAuthority lowerRole = rolesToVisitSet.iterator().next();
rolesToVisitSet.remove(aRole); rolesToVisitSet.remove(lowerRole);
if (!visitedRolesSet.add(aRole) || !this.rolesReachableInOneStepMap.containsKey(aRole)) { if (!visitedRolesSet.add(lowerRole) ||
!this.rolesReachableInOneStepMap.containsKey(lowerRole.getAuthority())) {
continue; // Already visited role or role with missing hierarchy continue; // Already visited role or role with missing hierarchy
} else if (role.equals(aRole)) { } else if (roleName.equals(lowerRole.getAuthority())) {
throw new CycleInRoleHierarchyException(); throw new CycleInRoleHierarchyException();
} }
rolesToVisitSet.addAll(this.rolesReachableInOneStepMap.get(aRole)); rolesToVisitSet.addAll(this.rolesReachableInOneStepMap.get(lowerRole.getAuthority()));
} }
this.rolesReachableInOneOrMoreStepsMap.put(role, visitedRolesSet); this.rolesReachableInOneOrMoreStepsMap.put(roleName, visitedRolesSet);
logger.debug("buildRolesReachableInOneOrMoreStepsMap() - From role " + role logger.debug("buildRolesReachableInOneOrMoreStepsMap() - From role " + roleName
+ " one can reach " + visitedRolesSet + " in one or more steps."); + " one can reach " + visitedRolesSet + " in one or more steps.");
} }