What is proposed is to use the same two pass authentication that is done in the original XWiki authentication against its DB, to the LDAP authentication. Therefore LDAP authentication is done that way:

• Try current wiki ldap authentication
• if failed, and not in Main Wiki, try ldap authentication based on the main wiki ldap config
• if failed and try_local is enable, try local wiki DB and main wiki DB

This model has the advantage to be simple to understand and generic like the original authentication against xwiki DB. It has one drawback, the first matching ldap authentication will drive the creation of the user in the xwiki DB. Therefore, to avoind duplicating main xwiki users into local xwiki DB, it is important that main xwiki users failed to authenticate using the local xwiki ldap authentication, do that fallback to main ldap xwiki authentication occurs.

To help achieving this goal, the patch introduce an additional group filter, that worked as an exclusion group:

In xwiki.cfg: xwiki.authentication.ldap.exclude_group=groupDN
In WebPreferences: ldap_exclude_group=groupDN (This field should be added to the XWikiPreferences class)

When user pass the user_group filter successfully (if available), it is that check against the exclude_group and if a match is found, authentication failed.

• in XWikiLDAPUtils#isUserInGroup :
  • you removed the complete Map<String, String> for Map
  • the debug log "Found user dn in user group:" + userDN is called even returned userDN is null (meaning not found in the group)
• in XWikiLDAPAuthServiceImpl:
  • in #authenticate
    • possible null pointer exception:

      if (context != null) {
      [...]
      } else {
        context.put("message", "loginfailed");
        return null;
      }
      

      If the context is null you will get a NullPointer exception and anyway context should never be null so best is to remove this test as it was IMO (the default authenticator is old and maybe somewhere far in the past the context could be null )

    • you should not remove spaces from user name, it makes impossible to log in LDAP with user containing space so it's a regression.
  • if LDAP login fail and trylocal=0, no error message are stored in the context (which makes XWiki.XWikiLogin reloading without saying anything) or even logged

To summarize:

• I don't agree with your copy/past from standard XWikiAuthServiceImpl (which contains old and sometimes wrong code). I think you should take the current XWikiLDAPAuthServiceImpl#authenticate and fix/change only what to be fixed for XWIKI-2495 and XWIKI-2515. Fix the empty user/password which does not print error in XWiki.XWikiLogin should be another jira issue and ideally a different patch.
• Except for this all seems ok to me as first look.

• In XWikiLDAPUtils#isUserInGroup
• Sorry for the Map, it was a regression I have overlooked from 1.4.1 - FIXED
• I have removed the debug log here because the caller better know what to say since this check is now used for exclude group as well, and since there is only one caller which also do its debug log, the debug message seems to me redundant and confusing when called for exclusion.

• In XWikiLDAPAuthServiceImpl
• in #authenticate
• I have removed the check of a null context has you suggest, but it would be nice to also remove this from XwikiAuthServiceImpl - see below as well
• You may have notice the comment and the FIXME, that said that the intended purpose was to remove space before and after the username, and my feel that it does more then that. I understand that you see it as a regression, so I have fix it has well, replacing replace by trim
• if LDAP fail, trylocal=0 and warn logging is not enabled or ldap failed with exception, existing code does not store a message in the context, so this is a separate issue, I feel that my regression is more consistant, since having side effect from logging is very poor.

To summarize:

• Until the standard, old, but not so poor original code is not improved, I feel that proposing something similar, with an initial cleaning phase, then a true authentication one, will help moving the cleaning phase upper, and simplify the whole thing at the end, since cleaning is redundant actually. This is a first step in that direction, maybe I will code the rest later, but you need to accept this patch first
• Regarding the proper message reporting, I suggest that you open a new issue to improve that point, I do not see my patch as a regression.

In the hope that you agree, here is my fixed patch.

• I'm not sure about the trim, LDAP can't contains uid with spaces before and after ?

The general problem is that it's difficult to apply a patch doing so many modifications not related with the issue. I don't speaking about improvement or not, simply this patch change behaviors from user point of view that is not referenced anywhere. What you did specifically for XWIKI-2495 and XWIKI-2515 seems perfect to me.