From 0db440a839de1f6bb263c089cdce678c1f241732 Mon Sep 17 00:00:00 2001 From: Brian Demers Date: Fri, 18 Mar 2022 17:24:33 -0400 Subject: [PATCH] Apply principalSuffix only when username does NOT already contain the suffix Fixes: SHIRO-871 --- .../activedirectory/ActiveDirectoryRealm.java | 2 +- .../shiro/realm/ldap/AbstractLdapRealm.java | 12 ++++ .../ActiveDirectoryRealmTest.java | 56 +++++++++++++++++++ 3 files changed, 69 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/apache/shiro/realm/activedirectory/ActiveDirectoryRealm.java b/core/src/main/java/org/apache/shiro/realm/activedirectory/ActiveDirectoryRealm.java index 39fa4b62b4..3dc459cb33 100644 --- a/core/src/main/java/org/apache/shiro/realm/activedirectory/ActiveDirectoryRealm.java +++ b/core/src/main/java/org/apache/shiro/realm/activedirectory/ActiveDirectoryRealm.java @@ -163,7 +163,7 @@ protected Set getRoleNamesForUser(String username, LdapContext ldapConte searchCtls.setSearchScope(SearchControls.SUBTREE_SCOPE); String userPrincipalName = username; - if (principalSuffix != null) { + if (principalSuffix != null && !userPrincipalName.toLowerCase(Locale.ROOT).endsWith(principalSuffix.toLowerCase(Locale.ROOT))) { userPrincipalName += principalSuffix; } diff --git a/core/src/main/java/org/apache/shiro/realm/ldap/AbstractLdapRealm.java b/core/src/main/java/org/apache/shiro/realm/ldap/AbstractLdapRealm.java index 25458c9eb9..36b2ec562a 100644 --- a/core/src/main/java/org/apache/shiro/realm/ldap/AbstractLdapRealm.java +++ b/core/src/main/java/org/apache/shiro/realm/ldap/AbstractLdapRealm.java @@ -63,6 +63,18 @@ public abstract class AbstractLdapRealm extends AuthorizingRealm { /*-------------------------------------------- | I N S T A N C E V A R I A B L E S | ============================================*/ + + /** + * Defines the Suffix added to the User Principal Name when looking up groups (e.g. "memberOf") + * AD Example: + * User's Principal Name be "John.Doe" + * User's E-Mail Address be "John.Doe@example.com" + * + * For the example below, set: + * realm.principalSuffix = @example.com + * + * Only then, "John.Doe" and also "John.Doe@example.com" can authorize against groups + */ protected String principalSuffix = null; protected String searchBase = null; diff --git a/core/src/test/java/org/apache/shiro/realm/activedirectory/ActiveDirectoryRealmTest.java b/core/src/test/java/org/apache/shiro/realm/activedirectory/ActiveDirectoryRealmTest.java index 5a2ce0e7ff..8d4b661e4f 100644 --- a/core/src/test/java/org/apache/shiro/realm/activedirectory/ActiveDirectoryRealmTest.java +++ b/core/src/test/java/org/apache/shiro/realm/activedirectory/ActiveDirectoryRealmTest.java @@ -24,6 +24,7 @@ import org.apache.shiro.authz.AuthorizationInfo; import org.apache.shiro.authz.SimpleAuthorizationInfo; import org.apache.shiro.mgt.DefaultSecurityManager; +import org.apache.shiro.mgt.SecurityManager; import org.apache.shiro.realm.AuthorizingRealm; import org.apache.shiro.realm.UserIdPrincipal; import org.apache.shiro.realm.UsernamePrincipal; @@ -32,14 +33,29 @@ import org.apache.shiro.subject.SimplePrincipalCollection; import org.apache.shiro.subject.Subject; import org.apache.shiro.util.ThreadContext; +import org.easymock.Capture; +import org.easymock.CaptureType; import org.junit.After; +import org.junit.Assert; import org.junit.Before; import org.junit.Test; +import javax.naming.NamingEnumeration; import javax.naming.NamingException; +import javax.naming.directory.SearchControls; +import javax.naming.directory.SearchResult; +import javax.naming.ldap.LdapContext; import java.util.HashSet; import java.util.Set; +import static org.easymock.EasyMock.anyObject; +import static org.easymock.EasyMock.anyString; +import static org.easymock.EasyMock.capture; +import static org.easymock.EasyMock.createMock; +import static org.easymock.EasyMock.expect; +import static org.easymock.EasyMock.replay; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.*; import static org.junit.Assert.assertTrue; @@ -97,6 +113,43 @@ public void testDefaultConfig() { subject.logout(); } + @Test + public void testExistingUserSuffix() throws Exception { + assertExistingUserSuffix(USERNAME, "testuser@ExAmple.COM"); // suffix case matches configure suffix + + // suffix matches user entry + assertExistingUserSuffix(USERNAME + "@example.com", "testuser@example.com"); + assertExistingUserSuffix(USERNAME + "@EXAMPLE.com", "testuser@EXAMPLE.com"); + } + + public void assertExistingUserSuffix(String username, String expectedPrincipalName) throws Exception { + + LdapContext ldapContext = createMock(LdapContext.class); + NamingEnumeration results = createMock(NamingEnumeration.class); + Capture captureArgs = Capture.newInstance(CaptureType.ALL); + expect(ldapContext.search(anyString(), anyString(), capture(captureArgs), anyObject(SearchControls.class))).andReturn(results); + replay(ldapContext); + + ActiveDirectoryRealm activeDirectoryRealm = new ActiveDirectoryRealm() {{ + this.principalSuffix = "@ExAmple.COM"; + }}; + + SecurityManager securityManager = new DefaultSecurityManager(activeDirectoryRealm); + Subject subject = new Subject.Builder(securityManager).buildSubject(); + subject.execute(() -> { + + try { + activeDirectoryRealm.getRoleNamesForUser(username, ldapContext); + } catch (NamingException e) { + Assert.fail("Unexpected NamingException thrown during test"); + } + }); + + Object[] args = captureArgs.getValue(); + assertThat(args, arrayWithSize(1)); + assertThat(args[0], is(expectedPrincipalName)); + } + public class TestActiveDirectoryRealm extends ActiveDirectoryRealm { /*-------------------------------------------- @@ -117,6 +170,9 @@ public boolean doCredentialsMatch(AuthenticationToken object, AuthenticationInfo setCredentialsMatcher(credentialsMatcher); } + public void setPrincipalSuffix(String principalSuffix) { + this.principalSuffix = principalSuffix; + } protected AuthenticationInfo doGetAuthenticationInfo(AuthenticationToken token) throws AuthenticationException { SimpleAccount account = (SimpleAccount) super.doGetAuthenticationInfo(token);