[xwiki-notifications] [Issue] Issue Comment Edited: (XWIKI-2495) LDAP Authentication re-create global user locally

[Thomas Mortagne (JIRA)]

    [ http://jira.xwiki.org/jira/browse/XWIKI-2495?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=27639#action_27639 ] 

Thomas Mortagne edited comment on XWIKI-2495 at 7/1/08 3:13 PM:
----------------------------------------------------------------

Some comment:
- 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:
{noformat}
if (context != null) {
[...]
} else {
  context.put("message", "loginfailed");
  return null;
}
{noformat} 
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.

      was (Author: tmortagne):
    Some comment:
- 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:
{noformat}
if (context != null) {
[...]
} else {
  context.put("message", "loginfailed");
  return null;
}
{noformat} 
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 and 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.
  
> LDAP Authentication re-create global user locally
> -------------------------------------------------
>
>                 Key: XWIKI-2495
>                 URL: http://jira.xwiki.org/jira/browse/XWIKI-2495
>             Project: XWiki Core
>          Issue Type: Bug
>          Components: Auth Service - LDAP
>    Affects Versions: 1.5 M1, 1.4.1
>            Reporter: Denis Gervalle
>            Assignee: Thomas Mortagne
>             Fix For: 1.5 RC1
>
>         Attachments: ldap-1.4.1-local-global-auth.patch, ldap-1.5M2-local-global-auth.patch
>
>
> When a global user logs in locally, a new local user is created instead of updating the global user which has rights to the local xwiki. This seems to be related to the way findUser() search for existing user only the local database.
> Moreover, restriction based on ldap.user_group applied locally, require global user to be also member of the locally defined ldap group to be able to log in. Do not know if this intended or not, but I do not feel this to be very practical since the only way I have got to avoid local user to login globally and create a user globally, is by restricting global and local login to different ldap group. 

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://jira.xwiki.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira