Copilot commented on code in PR #2854:
URL: https://github.com/apache/shiro/pull/2854#discussion_r3764217760
##########
core/src/test/java/org/apache/shiro/realm/activedirectory/ActiveDirectoryRealmTest.java:
##########
@@ -149,6 +152,42 @@ void testInitialization() {
SecurityUtils.setSecurityManager(null);
}
+ @Test
+ void testUsernameForAuthenticationWithDn() {
+ ActiveDirectoryRealm activeDirectoryRealm = new ActiveDirectoryRealm();
Review Comment:
This DN-preservation test currently exercises the default realm where
`principalSuffix` is null. Since the reported bug involves escaping/appending
the configured suffix, it would be good to set a non-null `principalSuffix`
here to ensure DN inputs are still preserved even when suffixing is enabled.
This issue also appears on line 182 of the same file.
##########
core/src/main/java/org/apache/shiro/realm/activedirectory/ActiveDirectoryRealm.java:
##########
@@ -109,7 +110,7 @@ protected AuthenticationInfo
queryForAuthenticationInfo(AuthenticationToken toke
// Binds using the username and password provided by the user.
LdapContext ctx = null;
try {
- ctx =
ldapContextFactory.getLdapContext(getUsernameWithSuffix(upToken.getUsername()),
+ ctx =
ldapContextFactory.getLdapContext(getUsernameForAuthentication(upToken.getUsername()),
String.valueOf(upToken.getPassword()));
Review Comment:
`queryForAuthenticationInfo` now binds with
`getUsernameForAuthentication(...)`, but DN-preserving logic is not applied
when resolving group memberships: `queryForAuthorizationInfo` passes the (raw)
principal into `getRoleNamesForUser`, which still calls
`getUsernameWithSuffix(username)`.
If a user authenticates with a DN while `principalSuffix` is configured, the
authorization path will escape/append the suffix to the DN and the LDAP search
will likely fail (roles won’t resolve for DN-authenticated users). Consider
reusing the same DN-detection logic when building search arguments (or
otherwise ensure the resolved/bind username is used consistently for both
authentication and authorization).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
