給spring-security提了一個issue

前言

今天在用spring-security的角色繼承時，遇到了一個坑，經過調試源碼解決了，而後發現這應該是spring-security自己的一個小問題，而後就在Spring官方的GitHub上提了一個issue

正文

我在使用spring-security的角色繼承，關鍵代碼片斷以下：

...
// 定義角色繼承，兩個角色繼承之間用空格或and鏈接
roleHierarchyImpl.setHierarchy("ROLE_DEVELOPER > ROLE_SUPERVISOR and ROLE_SUPERVISOR > ROLE_ADMIN and ROLE_ADMIN > ROLE_USER and ROLE_USER > ROLE_ANONYMOUS");
...
複製代碼

...
// 定義須要的權限表達式
.access("hasRole('USER')")
...
複製代碼

上邊關於角色繼承的定義方式，是我在使用以前版本的spring-security得到經驗，同時，經過spring-security源碼的註釋也可看到相關說明

/** * The simple interface of a role hierarchy. * * @author Michael Mayr */
public interface RoleHierarchy {

   /** * Returns an array of all reachable authorities. * <p> * Reachable authorities are the directly assigned authorities plus all authorities * that are (transitively) reachable from them in the role hierarchy. * <p> * Example:<br> * Role hierarchy: ROLE_A &gt; ROLE_B and ROLE_B &gt; ROLE_C.<br> * Directly assigned authority: ROLE_A.<br> * Reachable authorities: ROLE_A, ROLE_B, ROLE_C. * * @param authorities - List of the directly assigned authorities. * @return List of all reachable authorities given the assigned authorities. */
   public Collection<? extends GrantedAuthority> getReachableGrantedAuthorities(
         Collection<? extends GrantedAuthority> authorities);

}
複製代碼

可是，當我實際跑起來後，發現根本不行，角色繼承沒生效。我就很納悶了，原來用過spring-security啊，就是這樣就能夠啊。而後試了改了改權限表達式，結果仍是不行，而後我想了想，調試源碼吧，調試源碼通常都是必殺技。在調試源碼的過程當中，我逐漸發現了問題所在。

我先給出角色表達式解析以及角色繼承解析的相關代碼路徑，你們可按這個路徑跟蹤。 角色表達式解析：

// 由上到下爲執行路徑，最上端是入口點
org.springframework.security.web.FilterChainProxy.VirtualFilterChain#doFilter

org.springframework.security.web.access.intercept.FilterSecurityInterceptor#doFilter

org.springframework.security.access.intercept.AbstractSecurityInterceptor#beforeInvocation

org.springframework.security.web.access.intercept.FilterSecurityInterceptor#invoke

org.springframework.security.access.AccessDecisionManager#decide

org.springframework.security.access.vote.AffirmativeBased#decide

org.springframework.security.web.access.expression.WebExpressionVoter#vote

org.springframework.security.access.expression.ExpressionUtils#evaluateAsBoolean

org.springframework.expression.spel.standard.SpelExpression#getValue(org.springframework.expression.EvaluationContext, java.lang.Class<T>)

org.springframework.expression.spel.ast.SpelNodeImpl#getTypedValue

org.springframework.expression.spel.ast.MethodReference#getValueInternal(org.springframework.expression.spel.ExpressionState)

org.springframework.expression.spel.ast.MethodReference#getCachedExecutor

org.springframework.expression.spel.support.ReflectiveMethodExecutor#execute

java.lang.reflect.Method#invoke

org.springframework.security.access.expression.SecurityExpressionRoot#hasRole

org.springframework.security.access.expression.SecurityExpressionRoot#hasAnyRole

org.springframework.security.access.expression.SecurityExpressionRoot#hasAnyAuthorityName

複製代碼

注意上邊執行路徑中的 java.lang.reflect.Method#invoke 實際上，權限控制表達式內部的原理是是用反射去執行對應的用於判斷是否有權限方法的，也就是執行 org.springframework.security.access.expression.SecurityExpressionRoot#hasRole

{%asset_img 2.png%}

執行到下圖中這裏後，返回的是false也就是受權未經過，沒有對應角色，當前擁有的角色是從org.springframework.security.access.hierarchicalroles.RoleHierarchy#getReachableGrantedAuthorities得到的，裏面並無須要的角色"ROLE"，所以天然就是false

{%asset_img 3.png%}

那麼爲何沒有呢，按照角色繼承的定義，應該可以有才對啊，這是咱們就須要看角色繼承表達式生成具體角色的邏輯了，這個邏輯的代碼路徑是這個：

org.springframework.security.access.hierarchicalroles.RoleHierarchyImpl#setHierarchy

org.springframework.security.access.hierarchicalroles.RoleHierarchyImpl#buildRolesReachableInOneStepMap

org.springframework.security.access.hierarchicalroles.RoleHierarchyImpl#buildRolesReachableInOneOrMoreStepsMap
複製代碼

經過跟蹤這些代碼，咱們從中能夠看出，實際上，正確的角色繼承表達式應該這樣定義：

...
roleHierarchyImpl.setHierarchy("ROLE_DEVELOPER > ROLE_SUPERVISOR > ROLE_ADMIN > ROLE_USER > ROLE_ANONYMOUS");
...
複製代碼

實際上定義角色繼承表達式的規則已經變了，然而，在spring-security代碼庫中的RoleHierarchy這個類的註釋，還保留着舊版的角色繼承表達式的定義方式的說明，這應當是代碼更新了可是註釋未更新，我按照以往的經驗以及註釋的說明去寫，結果掉坑裏了。

總結

經過此次問題的排查，能夠說明：必要的註釋能夠有，可是不要過度依賴於註釋，要相信代碼自己，此外在此次調試源碼的過程當中我還發現了一個調試源碼的技巧：利用Drop Frame，能夠倒推代碼的執行路徑。

20190613更新

後來我發現，在4.2.x的spring-security中，角色繼承表達式不單單能夠用"and"鏈接符，它用任何一種鏈接符均可以。如下爲我在issue page上與@rwinch的對話：

Thanks for the clarification. RoleHiearchy isn't implementation specific. Instead it is trying to convey information rather than the configuration. That said, I can see how it might lead to confusion. Can you think of a way that makes the Javadoc in RoleHiearchy read better? If you do have a better wording, would you be willing to open a PR to change the RoleHiearchy Javadoc?

Or, to put it another way, perhaps the current version of "RoleHierarchyImpl" is not compatible with the definition rules of the old version of the "role inheritance" expression.

Can you clarify why you believe RoleHierarchyImpl worked differently and when it did? The code has gone largely untouched for over 10 years.

I reviewed my previous code these two days and learned that I used the version 4.2.2 before. Then, I looked at the source code for 4.2.2 and found out why the "and" connector can be used in the "role inheritance" expression in this version. In fact, in this version, the connector can be any string. Let's look at the code for details:

/** * 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. */
	private void buildRolesReachableInOneStepMap() {
		Pattern pattern = Pattern.compile("(\\s*([^\\s>]+)\\s*>\\s*([^\\s>]+))");

		Matcher roleHierarchyMatcher = pattern
				.matcher(this.roleHierarchyStringRepresentation);
		this.rolesReachableInOneStepMap = new HashMap<GrantedAuthority, Set<GrantedAuthority>>();

		while (roleHierarchyMatcher.find()) {
			GrantedAuthority higherRole = new SimpleGrantedAuthority(
					roleHierarchyMatcher.group(2));
			GrantedAuthority lowerRole = new SimpleGrantedAuthority(
					roleHierarchyMatcher.group(3));
			Set<GrantedAuthority> rolesReachableInOneStepSet;

			if (!this.rolesReachableInOneStepMap.containsKey(higherRole)) {
				rolesReachableInOneStepSet = new HashSet<GrantedAuthority>();
				this.rolesReachableInOneStepMap.put(higherRole,
						rolesReachableInOneStepSet);
			}
			else {
				rolesReachableInOneStepSet = this.rolesReachableInOneStepMap
						.get(higherRole);
			}
			addReachableRoles(rolesReachableInOneStepSet, lowerRole);

			logger.debug("buildRolesReachableInOneStepMap() - From role " + higherRole
					+ " one can reach role " + lowerRole + " in one step.");
		}
	}
複製代碼

In this code, a regular expression grouping match is used to find a group that matches the rule. In fact, the string to be matched can contain any kind of connector. Any kind of connector will not affect the result of the expression "roleHierarchyMatcher.find()" equal to true.

@Test
	public void testRegexForRoleHierarchyString() {
		Pattern pattern = Pattern.compile("(\\s*([^\\s>]+)\\s*>\\s*([^\\s>]+))");

		String roleHierarchyStringSplitByAnd = "ROLE_HIGHEST > ROLE_HIGHER and ROLE_HIGHER > ROLE_LOW and ROLE_LOW > ROLE_LOWER";
		String roleHierarchyStringSplitByOr = "ROLE_HIGHEST > ROLE_HIGHER or ROLE_HIGHER > ROLE_LOW or ROLE_LOW > ROLE_LOWER";
		String roleHierarchyStringSplitBySpace = "ROLE_HIGHEST > ROLE_HIGHER ROLE_HIGHER > ROLE_LOW ROLE_LOW > ROLE_LOWER";
		String roleHierarchyStringSplitByWhatever = "ROLE_HIGHEST > ROLE_HIGHER xxx ROLE_HIGHER > ROLE_LOW xxx ROLE_LOW > ROLE_LOWER";

		List<String> roleHierarchyStrings = new LinkedList<String>();
		roleHierarchyStrings.add(roleHierarchyStringSplitByAnd);
		roleHierarchyStrings.add(roleHierarchyStringSplitByOr);
		roleHierarchyStrings.add(roleHierarchyStringSplitBySpace);
		roleHierarchyStrings.add(roleHierarchyStringSplitByWhatever);

		for (String roleHierarchyString : roleHierarchyStrings) {
			Matcher roleHierarchyMatcher = pattern.matcher(roleHierarchyString);
			if (!roleHierarchyMatcher.find()) {
				throw new RuntimeException("I'm dead. X﹏X");
			}
		}
		System.out.println("All pass");
	}
複製代碼

All pass

Process finished with exit code 0
複製代碼

That is to say, in the version 4.2.2 or the adjacent version, the "RoleHierarchy" comment is neither an error nor a correct one, XD.

/** * The simple interface of a role hierarchy. * * @author Michael Mayr */
public interface RoleHierarchy {

	/** * Returns an array of all reachable authorities. * <p> * Reachable authorities are the directly assigned authorities plus all authorities * that are (transitively) reachable from them in the role hierarchy. * <p> * Example:<br> * Role hierarchy: ROLE_A &gt; ROLE_B and ROLE_B &gt; ROLE_C.<br> * Directly assigned authority: ROLE_A.<br> * Reachable authorities: ROLE_A, ROLE_B, ROLE_C. * * @param authorities - List of the directly assigned authorities. * @return List of all reachable authorities given the assigned authorities. */
	public Collection<? extends GrantedAuthority> getReachableGrantedAuthorities(
			Collection<? extends GrantedAuthority> authorities);

}
複製代碼