Sorry, in the above code the section
if (name != null) {
try {
if (debug) {
log.debug("Cache hit. Using DN " + name);
}
bValue = this.bindAsUser(userName,name, (String)
credential);
} catch (NamingException e) {
// do nothing if bind fails since we check for other DN
// patterns as well.
if (log.isDebugEnabled()) {
log.debug("Checking authentication with UserDN " + name
+ "failed " +
e.getMessage(), e);
}
}
return bValue;
}
should be changed as
if (name != null) {
try {
if (debug) {
log.debug("Cache hit. Using DN " + name);
}
bValue = this.bindAsUser(userName,name, (String)
credential);
} catch (NamingException e) {
// do nothing if bind fails since we check for other DN
// patterns as well.
if (log.isDebugEnabled()) {
log.debug("Checking authentication with UserDN " + name
+ "failed " +
e.getMessage(), e);
}
}
if(bValue){
return bValue;
}
}
On Thu, Jun 25, 2015 at 6:11 PM, Damith Senanayake <[email protected]> wrote:
> in org.wso2.carbon.user.core.ldap.ReadOnlyLDAPUserStoreManager.java, the
> following section follows a logic as
>
> 1. Check cache for user RDN
> -a) if RDN in cache, try authentication with User Store, and return true
> if successful,
> -b) if RDN in cache and authentication fails, continue,
>
> 2. Try creating user RDN from UserDN Patterns, and attempt authentication
> 3. If UserDN patterns are null, try authentication from UserBase, and
> attempt authentication.
>
> The code is as below
>
> boolean debug = log.isDebugEnabled();
>
> if (userName == null || credential == null) {
> return false;
> }
>
> userName = userName.trim();
>
> String password = (String) credential;
> password = password.trim();
>
> if (userName.equals("") || password.equals("")) {
> return false;
> }
>
> if (debug) {
> log.debug("Authenticating user " + userName);
> }
>
> boolean bValue = false;
> // check cached user DN first.
> String name = userCache.get(userName);
> if (name != null) {
> try {
> if (debug) {
> log.debug("Cache hit. Using DN " + name);
> }
> bValue = this.bindAsUser(userName,name, (String)
> credential);
> } catch (NamingException e) {
> // do nothing if bind fails since we check for other DN
> // patterns as well.
> if (log.isDebugEnabled()) {
> log.debug("Checking authentication with UserDN " +
> name + "failed " +
> e.getMessage(), e);
> }
> }
>
> return bValue;
>
> }
>
> // read list of patterns from user-mgt.xml
> String patterns =
> realmConfig.getUserStoreProperty(LDAPConstants.USER_DN_PATTERN);
>
> if (patterns != null && !patterns.isEmpty()) {
>
> if (debug) {
> log.debug("Using UserDNPatterns " + patterns);
> }
>
> // if the property is present, split it using # to see if
> there are
> // multiple patterns specified.
> String[] userDNPatternList = patterns.split("#");
> if (userDNPatternList.length > 0) {
> for (String userDNPattern : userDNPatternList) {
> name = MessageFormat.format(userDNPattern,
>
> escapeUsernameSpecialCharacters(userName,true));
> if (debug) {
> log.debug("Authenticating with " + name);
> }
> try {
> if (name != null) {
> bValue = this.bindAsUser(userName, name,
> (String) credential);
> if (bValue) {
> userCache.put(userName, name);
> break;
> }
> }
> } catch (NamingException e) {
> // do nothing if bind fails since we check for
> other DN
> // patterns as well.
> if (log.isDebugEnabled()) {
> log.debug("Checking authentication with UserDN
> " + userDNPattern +
> "failed " + e.getMessage(), e);
> }
> }
> }
> }
> } else {
> name = getNameInSpaceForUserName(userName);
> try {
> if (name != null) {
> if (debug) {
> log.debug("Authenticating with " + name);
> }
> bValue = this.bindAsUser(userName, name, (String)
> credential);
> if (bValue) {
> userCache.put(userName, name);
> }
> }
> } catch (NamingException e) {
> String errorMessage = "Cannot bind user : " + userName;
> if (log.isDebugEnabled()) {
> log.debug(errorMessage, e);
> }
> throw new UserStoreException(errorMessage, e);
> }
> }
>
> return bValue;
>
> For every failed attempt, two of the highlighted bindings will occur.
>
> By following this logic, in the case of a failed attempt, we make two
> authentication requests to the LDAP/AD user-store (as both
> ReadWriteLDAPUserStoreManager.java and ActiveDirectoryUserStoreManager.java
> are both extended from this).
> If a user configures an AD to lock users on failed attempts (not by
> configuring the IS or the Identity management policy), each failed attempt
> would account for two failed attempts although the user will only see one
> (this means the account will be locked in half the allowed attempts for the
> users).
>
> Why do we need to check again if the authentication fails when we draw
> from the cache? Couldn't we remove this redundancy by either removing the
> redundant check if the authentication from cached username fails or avoid
> checking the cache altogether?
>
> Thanks in advance
>
>
>
> --
> *-Damith Senanayake-*
> +94712205272
>
--
*-Damith Senanayake-*
+94712205272
_______________________________________________
Dev mailing list
[email protected]
http://wso2.org/cgi-bin/mailman/listinfo/dev
