Skip to content

Commit 422166d

Browse files
bdemersbdemers
authored
Merge pull request #364 from apache/shiro-871-19x
Apply principalSuffix only when username does NOT already contain the suffix
2 parents a259d26 + 0db440a commit 422166d

File tree

3 files changed

+69
-1
lines changed

3 files changed

+69
-1
lines changed

‎core/src/main/java/org/apache/shiro/realm/activedirectory/ActiveDirectoryRealm.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ protected Set<String> getRoleNamesForUser(String username, LdapContext ldapConte
163163
searchCtls.setSearchScope(SearchControls.SUBTREE_SCOPE);
164164

165165
String userPrincipalName = username;
166-
if (principalSuffix != null) {
166+
if (principalSuffix != null && !userPrincipalName.toLowerCase(Locale.ROOT).endsWith(principalSuffix.toLowerCase(Locale.ROOT))) {
167167
userPrincipalName += principalSuffix;
168168
}
169169

‎core/src/main/java/org/apache/shiro/realm/ldap/AbstractLdapRealm.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,18 @@ public abstract class AbstractLdapRealm extends AuthorizingRealm {
6363
/*--------------------------------------------
6464
| I N S T A N C E V A R I A B L E S |
6565
============================================*/
66+
67+
/**
68+
* Defines the Suffix added to the User Principal Name when looking up groups (e.g. "memberOf")
69+
* AD Example:
70+
* User's Principal Name be "John.Doe"
71+
* User's E-Mail Address be "[email protected]"
72+
*
73+
* For the example below, set:
74+
* realm.principalSuffix = @example.com
75+
*
76+
* Only then, "John.Doe" and also "[email protected]" can authorize against groups
77+
*/
6678
protected String principalSuffix = null;
6779

6880
protected String searchBase = null;

‎core/src/test/java/org/apache/shiro/realm/activedirectory/ActiveDirectoryRealmTest.java‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import org.apache.shiro.authz.AuthorizationInfo;
2525
import org.apache.shiro.authz.SimpleAuthorizationInfo;
2626
import org.apache.shiro.mgt.DefaultSecurityManager;
27+
import org.apache.shiro.mgt.SecurityManager;
2728
import org.apache.shiro.realm.AuthorizingRealm;
2829
import org.apache.shiro.realm.UserIdPrincipal;
2930
import org.apache.shiro.realm.UsernamePrincipal;
@@ -32,14 +33,29 @@
3233
import org.apache.shiro.subject.SimplePrincipalCollection;
3334
import org.apache.shiro.subject.Subject;
3435
import org.apache.shiro.util.ThreadContext;
36+
import org.easymock.Capture;
37+
import org.easymock.CaptureType;
3538
import org.junit.After;
39+
import org.junit.Assert;
3640
import org.junit.Before;
3741
import org.junit.Test;
3842

43+
import javax.naming.NamingEnumeration;
3944
import javax.naming.NamingException;
45+
import javax.naming.directory.SearchControls;
46+
import javax.naming.directory.SearchResult;
47+
import javax.naming.ldap.LdapContext;
4048
import java.util.HashSet;
4149
import java.util.Set;
4250

51+
import static org.easymock.EasyMock.anyObject;
52+
import static org.easymock.EasyMock.anyString;
53+
import static org.easymock.EasyMock.capture;
54+
import static org.easymock.EasyMock.createMock;
55+
import static org.easymock.EasyMock.expect;
56+
import static org.easymock.EasyMock.replay;
57+
import static org.hamcrest.MatcherAssert.assertThat;
58+
import static org.hamcrest.Matchers.*;
4359
import static org.junit.Assert.assertTrue;
4460

4561

@@ -97,6 +113,43 @@ public void testDefaultConfig() {
97113
subject.logout();
98114
}
99115

116+
@Test
117+
public void testExistingUserSuffix() throws Exception {
118+
assertExistingUserSuffix(USERNAME, "[email protected]"); // suffix case matches configure suffix
119+
120+
// suffix matches user entry
121+
assertExistingUserSuffix(USERNAME + "@example.com", "[email protected]");
122+
assertExistingUserSuffix(USERNAME + "@EXAMPLE.com", "[email protected]");
123+
}
124+
125+
public void assertExistingUserSuffix(String username, String expectedPrincipalName) throws Exception {
126+
127+
LdapContext ldapContext = createMock(LdapContext.class);
128+
NamingEnumeration<SearchResult> results = createMock(NamingEnumeration.class);
129+
Capture<Object[]> captureArgs = Capture.newInstance(CaptureType.ALL);
130+
expect(ldapContext.search(anyString(), anyString(), capture(captureArgs), anyObject(SearchControls.class))).andReturn(results);
131+
replay(ldapContext);
132+
133+
ActiveDirectoryRealm activeDirectoryRealm = new ActiveDirectoryRealm() {{
134+
this.principalSuffix = "@ExAmple.COM";
135+
}};
136+
137+
SecurityManager securityManager = new DefaultSecurityManager(activeDirectoryRealm);
138+
Subject subject = new Subject.Builder(securityManager).buildSubject();
139+
subject.execute(() -> {
140+
141+
try {
142+
activeDirectoryRealm.getRoleNamesForUser(username, ldapContext);
143+
} catch (NamingException e) {
144+
Assert.fail("Unexpected NamingException thrown during test");
145+
}
146+
});
147+
148+
Object[] args = captureArgs.getValue();
149+
assertThat(args, arrayWithSize(1));
150+
assertThat(args[0], is(expectedPrincipalName));
151+
}
152+
100153
public class TestActiveDirectoryRealm extends ActiveDirectoryRealm {
101154

102155
/*--------------------------------------------
@@ -117,6 +170,9 @@ public boolean doCredentialsMatch(AuthenticationToken object, AuthenticationInfo
117170
setCredentialsMatcher(credentialsMatcher);
118171
}
119172

173+
public void setPrincipalSuffix(String principalSuffix) {
174+
this.principalSuffix = principalSuffix;
175+
}
120176

121177
protected AuthenticationInfo doGetAuthenticationInfo(AuthenticationToken token) throws AuthenticationException {
122178
SimpleAccount account = (SimpleAccount) super.doGetAuthenticationInfo(token);

0 commit comments

Comments
 (0)