Skip to content

Commit caff3ee

Browse files
tekultekul
committed
SEC-1231: Authentication.getAuthorities should be of type Collection<GrantedAuthority> and not List<GrantedAuthority>. Refactored the interface and related classes to match (UserDetails etc).
1 parent 07d7c0d commit caff3ee

File tree

64 files changed

+296
-355
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

64 files changed

+296
-355
lines changed

‎acl/src/main/java/org/springframework/security/acls/domain/AclAuthorizationStrategyImpl.java

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -98,12 +98,8 @@ public void securityCheck(Acl acl, int changeType) {
9898
}
9999

100100
// Iterate this principal's authorities to determine right
101-
List<GrantedAuthority> auths = authentication.getAuthorities();
102-
103-
for (int i = 0; i < auths.size(); i++) {
104-
if (requiredAuthority.equals(auths.get(i))) {
105-
return;
106-
}
101+
if (authentication.getAuthorities().contains(requiredAuthority)) {
102+
return;
107103
}
108104

109105
// Try to get permission via ACEs within the ACL

‎acl/src/main/java/org/springframework/security/acls/domain/SidRetrievalStrategyImpl.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
package org.springframework.security.acls.domain;
1717

1818
import java.util.ArrayList;
19+
import java.util.Collection;
1920
import java.util.List;
2021

2122
import org.springframework.security.access.hierarchicalroles.NullRoleHierarchy;
@@ -51,7 +52,7 @@ public SidRetrievalStrategyImpl(RoleHierarchy roleHierarchy) {
5152
//~ Methods ========================================================================================================
5253

5354
public List<Sid> getSids(Authentication authentication) {
54-
List<GrantedAuthority> authorities = roleHierarchy.getReachableGrantedAuthorities(authentication.getAuthorities());
55+
Collection<GrantedAuthority> authorities = roleHierarchy.getReachableGrantedAuthorities(authentication.getAuthorities());
5556
List<Sid> sids = new ArrayList<Sid>(authorities.size() + 1);
5657

5758
sids.add(new PrincipalSid(authentication));

‎cas/src/main/java/org/springframework/security/cas/authentication/CasAuthenticationToken.java

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -15,17 +15,14 @@
1515

1616
package org.springframework.security.cas.authentication;
1717

18-
import org.jasig.cas.client.validation.Assertion;
18+
import java.io.Serializable;
19+
import java.util.Collection;
1920

21+
import org.jasig.cas.client.validation.Assertion;
2022
import org.springframework.security.authentication.AbstractAuthenticationToken;
2123
import org.springframework.security.core.GrantedAuthority;
2224
import org.springframework.security.core.userdetails.UserDetails;
2325

24-
25-
import java.io.Serializable;
26-
import java.util.Arrays;
27-
import java.util.List;
28-
2926
/**
3027
* Represents a successful CAS <code>Authentication</code>.
3128
*
@@ -45,14 +42,6 @@ public class CasAuthenticationToken extends AbstractAuthenticationToken implemen
4542

4643
//~ Constructors ===================================================================================================
4744

48-
/**
49-
* @deprecated
50-
*/
51-
public CasAuthenticationToken(final String key, final Object principal, final Object credentials,
52-
final GrantedAuthority[] authorities, final UserDetails userDetails, final Assertion assertion) {
53-
this(key, principal, credentials, Arrays.asList(authorities), userDetails, assertion);
54-
}
55-
5645
/**
5746
* Constructor.
5847
*
@@ -71,7 +60,7 @@ public CasAuthenticationToken(final String key, final Object principal, final Ob
7160
* @throws IllegalArgumentException if a <code>null</code> was passed
7261
*/
7362
public CasAuthenticationToken(final String key, final Object principal, final Object credentials,
74-
final List<GrantedAuthority> authorities, final UserDetails userDetails, final Assertion assertion) {
63+
final Collection<GrantedAuthority> authorities, final UserDetails userDetails, final Assertion assertion) {
7564
super(authorities);
7665

7766
if ((key == null) || ("".equals(key)) || (principal == null) || "".equals(principal) || (credentials == null)

‎cas/src/test/java/org/springframework/security/cas/authentication/CasAuthenticationProviderTests.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,8 +98,8 @@ public void statefulAuthenticationIsSuccessful() throws Exception {
9898
CasAuthenticationToken casResult = (CasAuthenticationToken) result;
9999
assertEquals(makeUserDetailsFromAuthoritiesPopulator(), casResult.getPrincipal());
100100
assertEquals("ST-123", casResult.getCredentials());
101-
assertEquals(new GrantedAuthorityImpl("ROLE_A"), casResult.getAuthorities().get(0));
102-
assertEquals(new GrantedAuthorityImpl("ROLE_B"), casResult.getAuthorities().get(1));
101+
assertTrue(casResult.getAuthorities().contains(new GrantedAuthorityImpl("ROLE_A")));
102+
assertTrue(casResult.getAuthorities().contains(new GrantedAuthorityImpl("ROLE_B")));
103103
assertEquals(cap.getKey().hashCode(), casResult.getKeyHash());
104104
assertEquals("details", casResult.getDetails());
105105

‎cas/src/test/java/org/springframework/security/cas/authentication/CasAuthenticationTokenTests.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,9 @@
2222
import org.jasig.cas.client.validation.Assertion;
2323
import org.jasig.cas.client.validation.AssertionImpl;
2424
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
25-
import org.springframework.security.cas.authentication.CasAuthenticationToken;
2625
import org.springframework.security.core.GrantedAuthority;
2726
import org.springframework.security.core.authority.AuthorityUtils;
27+
import org.springframework.security.core.authority.GrantedAuthorityImpl;
2828
import org.springframework.security.core.userdetails.User;
2929
import org.springframework.security.core.userdetails.UserDetails;
3030

@@ -109,8 +109,8 @@ public void testGetters() {
109109
assertEquals("key".hashCode(), token.getKeyHash());
110110
assertEquals(makeUserDetails(), token.getPrincipal());
111111
assertEquals("Password", token.getCredentials());
112-
assertEquals("ROLE_ONE", token.getAuthorities().get(0).getAuthority());
113-
assertEquals("ROLE_TWO", token.getAuthorities().get(1).getAuthority());
112+
assertTrue(token.getAuthorities().contains(new GrantedAuthorityImpl("ROLE_ONE")));
113+
assertTrue(token.getAuthorities().contains(new GrantedAuthorityImpl("ROLE_TWO")));
114114
assertEquals(assertion, token.getAssertion());
115115
assertEquals(makeUserDetails().getUsername(), token.getUserDetails().getUsername());
116116
}

‎core/src/main/java/org/springframework/security/access/expression/SecurityExpressionRoot.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
package org.springframework.security.access.expression;
22

3+
import java.util.Collection;
34
import java.util.HashSet;
4-
import java.util.List;
55
import java.util.Set;
66

77
import org.springframework.security.access.hierarchicalroles.RoleHierarchy;
@@ -96,7 +96,7 @@ public void setRoleHierarchy(RoleHierarchy roleHierarchy) {
9696
private Set<String> getAuthoritySet() {
9797
if (roles == null) {
9898
roles = new HashSet<String>();
99-
List<GrantedAuthority> userAuthorities = authentication.getAuthorities();
99+
Collection<GrantedAuthority> userAuthorities = authentication.getAuthorities();
100100

101101
if (roleHierarchy != null) {
102102
userAuthorities = roleHierarchy.getReachableGrantedAuthorities(userAuthorities);

‎core/src/main/java/org/springframework/security/access/hierarchicalroles/NullRoleHierarchy.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
package org.springframework.security.access.hierarchicalroles;
22

3-
import java.util.List;
3+
import java.util.Collection;
44

55
import org.springframework.security.core.GrantedAuthority;
66

@@ -12,7 +12,7 @@
1212
*/
1313
public final class NullRoleHierarchy implements RoleHierarchy {
1414

15-
public List<GrantedAuthority> getReachableGrantedAuthorities(List<GrantedAuthority> authorities) {
15+
public Collection<GrantedAuthority> getReachableGrantedAuthorities(Collection<GrantedAuthority> authorities) {
1616
return authorities;
1717
}
1818

‎core/src/main/java/org/springframework/security/access/hierarchicalroles/RoleHierarchy.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414

1515
package org.springframework.security.access.hierarchicalroles;
1616

17-
import java.util.List;
17+
import java.util.Collection;
1818

1919
import org.springframework.security.core.GrantedAuthority;
2020

@@ -40,6 +40,6 @@ public interface RoleHierarchy {
4040
* @param authorities - List of the directly assigned authorities.
4141
* @return List of all reachable authorities given the assigned authorities.
4242
*/
43-
public List<GrantedAuthority> getReachableGrantedAuthorities(List<GrantedAuthority> authorities);
43+
public Collection<GrantedAuthority> getReachableGrantedAuthorities(Collection<GrantedAuthority> authorities);
4444

4545
}

‎core/src/main/java/org/springframework/security/access/hierarchicalroles/RoleHierarchyImpl.java

Lines changed: 45 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,21 @@
1515
package org.springframework.security.access.hierarchicalroles;
1616

1717

18-
import org.springframework.security.core.GrantedAuthority;
19-
import org.springframework.security.core.authority.GrantedAuthorityImpl;
20-
import org.apache.commons.logging.Log;
21-
import org.apache.commons.logging.LogFactory;
22-
18+
import java.util.ArrayList;
19+
import java.util.Collection;
20+
import java.util.HashMap;
21+
import java.util.HashSet;
22+
import java.util.Iterator;
23+
import java.util.List;
24+
import java.util.Map;
25+
import java.util.Set;
2326
import java.util.regex.Matcher;
2427
import java.util.regex.Pattern;
2528

26-
import java.util.*;
29+
import org.apache.commons.logging.Log;
30+
import org.apache.commons.logging.LogFactory;
31+
import org.springframework.security.core.GrantedAuthority;
32+
import org.springframework.security.core.authority.GrantedAuthorityImpl;
2733

2834
/**
2935
* <p>
@@ -98,7 +104,7 @@ public void setHierarchy(String roleHierarchyStringRepresentation) {
98104
buildRolesReachableInOneOrMoreStepsMap();
99105
}
100106

101-
public List<GrantedAuthority> getReachableGrantedAuthorities(List<GrantedAuthority> authorities) {
107+
public Collection<GrantedAuthority> getReachableGrantedAuthorities(Collection<GrantedAuthority> authorities) {
102108
if (authorities == null || authorities.isEmpty()) {
103109
return null;
104110
}
@@ -125,40 +131,40 @@ public List<GrantedAuthority> getReachableGrantedAuthorities(List<GrantedAuthori
125131
}
126132

127133
// SEC-863
128-
private void addReachableRoles(Set<GrantedAuthority> reachableRoles,
129-
GrantedAuthority authority) {
130-
131-
Iterator<GrantedAuthority> iterator = reachableRoles.iterator();
132-
while (iterator.hasNext()) {
133-
GrantedAuthority testAuthority = iterator.next();
134-
String testKey = testAuthority.getAuthority();
135-
if ((testKey != null) && (testKey.equals(authority.getAuthority()))) {
136-
return;
137-
}
138-
}
139-
reachableRoles.add(authority);
140-
}
134+
private void addReachableRoles(Set<GrantedAuthority> reachableRoles,
135+
GrantedAuthority authority) {
136+
137+
Iterator<GrantedAuthority> iterator = reachableRoles.iterator();
138+
while (iterator.hasNext()) {
139+
GrantedAuthority testAuthority = iterator.next();
140+
String testKey = testAuthority.getAuthority();
141+
if ((testKey != null) && (testKey.equals(authority.getAuthority()))) {
142+
return;
143+
}
144+
}
145+
reachableRoles.add(authority);
146+
}
141147

142148
// SEC-863
143-
private Set<GrantedAuthority> getRolesReachableInOneOrMoreSteps(
144-
GrantedAuthority authority) {
145-
146-
if (authority.getAuthority() == null) {
147-
return null;
148-
}
149-
150-
Iterator<GrantedAuthority> iterator = rolesReachableInOneOrMoreStepsMap.keySet().iterator();
151-
while (iterator.hasNext()) {
152-
GrantedAuthority testAuthority = iterator.next();
153-
String testKey = testAuthority.getAuthority();
154-
if ((testKey != null) && (testKey.equals(authority.getAuthority()))) {
155-
return rolesReachableInOneOrMoreStepsMap.get(testAuthority);
156-
}
157-
}
158-
159-
return null;
160-
}
161-
149+
private Set<GrantedAuthority> getRolesReachableInOneOrMoreSteps(
150+
GrantedAuthority authority) {
151+
152+
if (authority.getAuthority() == null) {
153+
return null;
154+
}
155+
156+
Iterator<GrantedAuthority> iterator = rolesReachableInOneOrMoreStepsMap.keySet().iterator();
157+
while (iterator.hasNext()) {
158+
GrantedAuthority testAuthority = iterator.next();
159+
String testKey = testAuthority.getAuthority();
160+
if ((testKey != null) && (testKey.equals(authority.getAuthority()))) {
161+
return rolesReachableInOneOrMoreStepsMap.get(testAuthority);
162+
}
163+
}
164+
165+
return null;
166+
}
167+
162168
/**
163169
* Parse input and build the map for the roles reachable in one step: the higher role will become a key that
164170
* references a set of the reachable lower roles.

‎core/src/main/java/org/springframework/security/access/hierarchicalroles/UserDetailsWrapper.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,9 @@
1414

1515
package org.springframework.security.access.hierarchicalroles;
1616

17-
import java.util.List;
17+
import java.util.Collection;
1818

19+
import org.springframework.security.access.vote.RoleHierarchyVoter;
1920
import org.springframework.security.core.GrantedAuthority;
2021
import org.springframework.security.core.userdetails.UserDetails;
2122

@@ -48,7 +49,7 @@ public boolean isAccountNonLocked() {
4849
return userDetails.isAccountNonLocked();
4950
}
5051

51-
public List<GrantedAuthority> getAuthorities() {
52+
public Collection<GrantedAuthority> getAuthorities() {
5253
return roleHierarchy.getReachableGrantedAuthorities(userDetails.getAuthorities());
5354
}
5455

@@ -72,4 +73,4 @@ public UserDetails getUnwrappedUserDetails() {
7273
return userDetails;
7374
}
7475

75-
}
76+
}

‎core/src/main/java/org/springframework/security/access/intercept/RunAsUserToken.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,7 @@
1616
package org.springframework.security.access.intercept;
1717

1818
import java.util.Arrays;
19-
import java.util.List;
20-
19+
import java.util.Collection;
2120

2221
import org.springframework.security.authentication.AbstractAuthenticationToken;
2322
import org.springframework.security.core.Authentication;
@@ -45,7 +44,7 @@ public RunAsUserToken(String key, Object principal, Object credentials, GrantedA
4544
this(key, principal, credentials, Arrays.asList(authorities), originalAuthentication);
4645
}
4746

48-
public RunAsUserToken(String key, Object principal, Object credentials, List<GrantedAuthority> authorities,
47+
public RunAsUserToken(String key, Object principal, Object credentials, Collection<GrantedAuthority> authorities,
4948
Class<? extends Authentication> originalAuthentication) {
5049
super(authorities);
5150
this.keyHash = key.hashCode();

‎core/src/main/java/org/springframework/security/access/vote/LabelBasedAclVoter.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import org.apache.commons.logging.LogFactory;
2424
import org.springframework.security.access.ConfigAttribute;
2525
import org.springframework.security.core.Authentication;
26+
import org.springframework.security.core.GrantedAuthority;
2627
import org.springframework.util.Assert;
2728

2829

@@ -177,8 +178,8 @@ public int vote(Authentication authentication, Object object, List<ConfigAttribu
177178
*/
178179
List<String> userLabels = new ArrayList<String>();
179180

180-
for (int i = 0; i < authentication.getAuthorities().size(); i++) {
181-
String userLabel = authentication.getAuthorities().get(i).getAuthority();
181+
for (GrantedAuthority authority : authentication.getAuthorities()) {
182+
String userLabel = authority.getAuthority();
182183
if (labelMap.containsKey(userLabel)) {
183184
userLabels.add(userLabel);
184185
logger.debug("Adding " + userLabel + " to <<<" + authentication.getName()

‎core/src/main/java/org/springframework/security/access/vote/RoleHierarchyVoter.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
package org.springframework.security.access.vote;
22

3-
import java.util.List;
3+
import java.util.Collection;
44

55
import org.springframework.security.access.hierarchicalroles.RoleHierarchy;
66
import org.springframework.security.core.Authentication;
@@ -26,7 +26,7 @@ public RoleHierarchyVoter(RoleHierarchy roleHierarchy) {
2626
* Calls the <tt>RoleHierarchy</tt> to obtain the complete set of user authorities.
2727
*/
2828
@Override
29-
List<GrantedAuthority> extractAuthorities(Authentication authentication) {
29+
Collection<GrantedAuthority> extractAuthorities(Authentication authentication) {
3030
return roleHierarchy.getReachableGrantedAuthorities(authentication.getAuthorities());
3131
}
3232
}

‎core/src/main/java/org/springframework/security/access/vote/RoleVoter.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
package org.springframework.security.access.vote;
1717

18+
import java.util.Collection;
1819
import java.util.List;
1920

2021
import org.springframework.security.access.AccessDecisionVoter;
@@ -94,7 +95,7 @@ public boolean supports(Class<?> clazz) {
9495

9596
public int vote(Authentication authentication, Object object, List<ConfigAttribute> attributes) {
9697
int result = ACCESS_ABSTAIN;
97-
List<GrantedAuthority> authorities = extractAuthorities(authentication);
98+
Collection<GrantedAuthority> authorities = extractAuthorities(authentication);
9899

99100
for (ConfigAttribute attribute : attributes) {
100101
if (this.supports(attribute)) {
@@ -112,7 +113,7 @@ public int vote(Authentication authentication, Object object, List<ConfigAttribu
112113
return result;
113114
}
114115

115-
List<GrantedAuthority> extractAuthorities(Authentication authentication) {
116+
Collection<GrantedAuthority> extractAuthorities(Authentication authentication) {
116117
return authentication.getAuthorities();
117118
}
118119
}

0 commit comments

Comments
 (0)