This has been done to support multiple DN pattern which is fixed for ticket [1]. If we just return the false, there can be some problem with it. Therefore we need to check whether multiple DN has been configured or not and then return the "false"
[1] https://wso2.org/jira/browse/CARBON-13631 Thanks, Asela. On Thu, Jun 25, 2015 at 6:14 PM, Damith Senanayake <[email protected]> wrote: > 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 -- Thanks & Regards, Asela ATL Mobile : +94 777 625 933 +358 449 228 979 http://soasecurity.org/ http://xacmlinfo.org/ _______________________________________________ Dev mailing list [email protected] http://wso2.org/cgi-bin/mailman/listinfo/dev
