[SHIRO-640] queryForAuthenticationInfo(): resolve DN using user name

[mephi42]

I am trying to use ActiveDirectoryRealm with searchFilter in order to make it possible to log in using e-mail (which is not part of DN). I see that this is partially supported in getRoleNamesForUser(), but not in queryForAuthenticationInfo().

This change make it fully work for me, but I have a feeling that it may disturb other users, so I'm willing to work on improving it.

[bdemers]

We may want to combine some of the functionality in the DefaultLdapRealm with the AD realm. Take a look at:

shiro/core/src/main/java/org/apache/shiro/realm/ldap/DefaultLdapRealm.java

Line 227 in 8acc82a

protected String getUserDn(String principal) throws IllegalArgumentException, IllegalStateException {

[mephi42]

Why are DefaultLdapRealm and AD realm are separated altogether?
Wouldn't it make sense to merge them?

[bdemers]

IIRC, ActiveDirectory typically uses one or two different schemas, depending on how it is configured. The schema for an LDAP server could be just about anything (but typically follow a couple patterns). We could make them share a lot more code through the AbstractLdapRealm

…

On Sat, Nov 25, 2017 at 6:25 PM, mephi42 ***@***.***> wrote: Why are DefaultLdapRealm and AD realm are separated altogether? Wouldn't it make sense to merge them? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#74 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAGGcnQM9ASnfsVDkSQKhAPi_JPG7bCsks5s6KHQgaJpZM4QjCnM> .

[mephi42]

In this update I made two improvements:

• shared user DN substitution logic between AbstractLdapRealm and DefaultLdapRealm
• took care of potential compatibility issue, when callers take care of resolution themselves, by providing a new userSearchFilter property

[rubenvanwanzeele]

@bdemers @mephi42 , did you think of a situation where the userDN cannot be composed by adding a prefix and a suffix to the username at all. So where authentication is handled with the userPrincipalName, but where the userDN is required for authorization. I haven't found a REALM implementation that resolves this issue.
The setup I'm handling is a AD one.
For making this more concrete:
The userPrincipalName format is johnthegreat@whatever.whatever.com
while the distinguishedName format is CN=John The Great,CN=Users,DC=whatever,DC=whatever,DC=com
The username is seen as {firstName}{lastName} (without the spaces), while the group membership (used for authorization) is handled with the userDN.

Currently I created a custom implementation for supporting this LDAP; but I was wondering whether I am the only one facing this problem. Instead of working with an userDnTemplate, I worked with an authenticationTemplate and a user search base. Using those, I was able to retrieve the userDN.
This approach still supports providing the userDN template as authenticationTemplate (I supposed this to be the case when the userSearchBase is not provided).

This is the config file I'm currently using:
ldapRealm = com.company.ldap.realm.CustomLdapRealm
ldapRealm.authenticationTemplate = {0}@whatever.whatever.com
ldapRealm.userSearchBase = CN=Users,DC=whatever,DC=whatever,DC=com
ldapRealm.userSearchFilter = (sAMAccountName={0})
ldapRealm.groupSearchBase = OU=Groups,DC=whatever,DC=whatever,DC=com
ldapRealm.groupSearchFilter = (member={0})
ldapRealm.groupSearchFilterAttribute = distinguishedName
ldapRealm.groupRoleAttribute = cn
ldapRealm.contextFactory.url = ldap://whatever.whatever.com:389
ldapRealm.contextFactory.systemUsername = system-user
ldapRealm.contextFactory.systemPassword = system-password

I have a local branch where I've implemented the changes in case you're willing to check them out.

Please be critic to what I've done, Your experience in this area might result in a better approach for handling this.

[bdemers]

@rubenvanwanzeele my running theory is that no two LDAP instances are the same :)

Put your code on a branch and add a link, it would be nice to merge in the common code if possible!

[bdemers]

Thanks @rubenvanwanzeele: #81

[rubenvanwanzeele]

@bdemers What's the status of this? Is there any update on this functionality?

[bdemers]

@rubenvanwanzeele the biggest thing I think we would need are tests.
It also looks like this change might add an additional LDAP query, though I haven't actually run it to verify that is the outcome in all cases.

Any help in either of those cases would be appreciated!