Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Users from AD groups starting with \ are not added in the XWiki mapped group #55

Closed
AndreeaChi opened this issue Jan 27, 2022 · 8 comments · Fixed by #56
Closed

Users from AD groups starting with \ are not added in the XWiki mapped group #55

AndreeaChi opened this issue Jan 27, 2022 · 8 comments · Fixed by #56
Assignees
Milestone

Comments

@AndreeaChi
Copy link

AndreeaChi commented Jan 27, 2022

Hi, I have tested in both 12.10.6 and 13.10.2 Jetty HSQLDB.
Steps to reproduce:

  1. Have an AD group called \#New Test with a user in it called XWikiUserOne

  2. Install AD app and activate ldap DEBUG mode in logging.

  3. Add an AD configuration in XWiki administration > Other > Active Directory

  4. Go on Users & Rights > Groups and create a group called BackslashDiez

  5. Associate the BackslashDiez group with \#New Test.
    The option to associate the groups is shown right next to the created XWiki groups.
    image
    backslashdiez-new-test-associate

  6. Check AD Advanced configuration, in the group mapping:

Expected result
XWiki.BackslashDiez > CN=\#New Test,CN=Users,DC=xwiki,DC=com

Actual result

  • the mapping with \#New Test was shown as on the test from 13.10.2 with triple backslashes
    XWiki.BackslashDiez > CN=\\\#New Test,CN=Users,DC=xwiki,DC=com
    Look-in-AD-Advanced-configuration
  1. Logged in with XWikiUserOne.

Expected result
Successful login, the user goes in the mapped group.

Actual result
It was a successful login, but the mapping was not done. The XWikiUserOne went in the XWikiAllGroup as done by default.
image

An extract of the logs showing the errors :
extract-logs-attempt-login-mapping-triple-backslash.txt

@AndreeaChi
Copy link
Author

AndreeaChi commented Jan 28, 2022

A second way to reproduce the failed mapping

  1. Tried as well without using the option "Associate" from XWiki Groups - wiki administration. I deleted the XWikiUserOne user page. Deleted the XWiki Groups: BackslashDiez and Slash and created a new one called SecondBackslashDiez and
  2. This time I did not click on the Associate button to associate it with an AD group. I went on the AD configuration > Advanced configuration and added manually the group mapping:
    `XWiki.SecondBackslashDiez > CN=#New Test,CN=Users,DC=xwiki,DC=com ' and Saved
  3. Tried to login with XWikiUserOne:

Expected result
CN=#New Test,CN=Users,DC=xwiki,DC=com
The login and mapping of XWikiUserOne are successful.

Actual result
The backslash is not shown next to the expected \#New Test group :
Looks like CN=#New Test,CN=Users,DC=xwiki,DC=com is not a DN, lets try filter or id
The login was done but the mapping was not successful, the XWiki.BackslashDiez group remained with 0 users. The XWikiUserOne went in the XWikiAllGroup as done by default.

Log extract:
extract-logs-attempt-login-manually-added-mapping-single-backslash.txt

@AndreeaChi
Copy link
Author

AndreeaChi commented Jan 28, 2022

An example of a successful mapping:

  1. Create on XWiki the group called Slash and in Active Directory Slash/InName
  2. In the AD group add the TestUser.
  3. Associate the XWiki Group Slash with the AD group.
    Associate-with-Slash-group
    The mapping is displayed as expected in AD app configuration page XWiki.Slash > CN=Slash/InName,CN=Users,DC=xwiki,DC=com and in the logs the frontslash is displayed correctly.
  4. Log with TestUser that is in the Slash Group.

Result:
The login and the mapping was successful. The TestUser went in the XWikiAllGroup as by default.
The XWiki.Slash group has also been updated:
image

Logs describing the successful mapping with the XWiki group XWiki.Slash :
2022-01-27 11:54:42,990 [qtp668849042-390 - http://localhost:8080/xwiki/bin/loginsubmit/XWiki/XWikiLogin] DEBUG o.x.c.l.XWikiLDAPUtils - Adding user [XWiki.TestUser] to xwiki group [XWiki.Slash] 2022-01-27 11:54:43,199 [qtp668849042-390 - http://localhost:8080/xwiki/bin/loginsubmit/XWiki/XWikiLogin] DEBUG o.x.c.l.XWikiLDAPUtils - Finished adding user [XWiki.TestUser] to xwiki group [XWiki.Slash]

@ClemensRobbenhaar
Copy link

The "triple backslashes" are actually ok in the display of the group; if the name is CN=\#New Test , then both the backslash and the hash sign must be escaped in the full 'DN' as:

  CN=\\\#New Test,CN=Users,DC=xwiki,DC=com

However when I try to get the mapping done, the group sync instead looks for a group:

  CN=\#New Test,CN=Users,DC=xwiki,DC=com

which is not what the mapping says. I have to bring in six backslashes to get the proper mapping.
So for some reason the proper escaping gets lost somewhere in between.

I have tested this with the plain "LDAP" connector, but from the logs it seems both this and the AD auth use the same group mapping code.

@AndreeaChi
Copy link
Author

AndreeaChi commented Feb 11, 2022

@ClemensRobbenhaar, thanks for testing and fixing the issue on the LDAP side: https://jira.xwiki.org/browse/LDAP-108

@ClemensRobbenhaar
Copy link

Thanks :)
I have released "LDAP Application 9.5.7" just now. Rebuilding the AD extension with this new version as dependency should also fix the issue here. If it does not, let me know so I can give it another try.

@AndreeaChi
Copy link
Author

AndreeaChi commented Feb 23, 2022

I have tried a test to update the LDAP Authenticator to the 9.5.7 version on my local Jetty 13.10.2 where the AD app 1.13.1 is installed and the issue still reproduces. I can login with the user from the group \#New Test but it does not get mapped in the XWiki group BackslashGroup.

Logs:
logs-trial-after-upgrade-LDAPAuthenticator-9.5.7.txt

@ClemensRobbenhaar
Copy link

Thanks for the feedback!

I have overlooked that the AD-Extension has a different way to handle the group mappings.
The variable aDValue defined in this line


and the separatedNewContent (and probably separatedOldContent)
var separatedOldContent = separator + oldContentToUpdate + separator;

need to be handled to escape / unescape the separator and backslash in a similar way as in xwiki-contrib/ldap@d525963

Unfortunately I have no working AD setup (and also I guess no write access to this repository), so I'd have a hard time contributing a fix for the issue in this project.

@trrenty
Copy link
Contributor

trrenty commented Mar 4, 2022

After some investigation I found out that the faulty function was getGroupMappings of XWikiLDAPConfig class.

I'm going to walk through an example using Andreea's data:

  • AD group called \#New Test with a user in it called XWikiUserOne
  • XWiki group called BackslashDiez

When we associated BackslashDiez with #New Test, the mapping was done successfully as
XWiki.BackslashDiez -> CN=\\\#New Test,CN=Users,DC=xwiki,DC=com (as Clemens stated) on windows server or
XWiki.BackslashDiez -> CN=\23New Test,CN=Users,DC=xwiki,DC=com on an OpenLDAP server.

Whenever we tried to perform an action that would eventually call getGroupMappings, it would return our mapping as CN=\#New Test,CN=Users,DC=xwiki,DC=com (windows) or CN=23New Test,CN=Users,DC=xwiki,DC=com respectively (OpenLDAP). Performing any LDAP query/action with these values, would fail.

I also created a group inside OpenLDAP called New\#Group but the name was automatically converted to New#Group, thus working properly. New\\Group on the other hand was affected. Behaviour on windows AD might differ.

The group mappings are saved as a concatenation of strings using the separator |. The faulty function tries to avoid confusion between the separator and possible occurrences of the symbol (|) within the dn of the groups.

I made this simple modification to the function and it fixed the problem. It also didn't affect the proper generation of the group mapping. I tested it on the LDAP server as well as the windows AD.

trrenty added a commit to trrenty/application-activedirectory that referenced this issue Mar 9, 2022
…d group xwikisas#55

* Escape the input values before storing them in the textarea
* Unescape the input values before displaying them at page load
* Updated the versions of LDAPUserImport and LDAP applications
trrenty added a commit that referenced this issue Apr 14, 2022
…d group #55 (#56)

* Users from AD groups starting with \ are not added in the XWiki mapped group #55

* Escape the input values before storing them in the textarea
* Unescape the input values before displaying them at page load
* Updated the versions of LDAPUserImport and LDAP applications

* Matched licensor version with upstream

* Updated ldauuserimport and ldap versions

* [Misc] Code changes

* Replaced escape and unescape functions with String.replaceAll

* Escape property value when removing a group mapping
@trrenty trrenty closed this as completed Apr 18, 2022
@trrenty trrenty self-assigned this Apr 18, 2022
@trrenty trrenty added this to the 1.14 milestone Apr 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants