Adambean / gitlab-ce-ldap-sync

Synchronise users and groups (including group members) from an LDAP instance with Gitlab CE (and EE in free tier) self-hosted instance(s).
Apache License 2.0
61 stars 23 forks source link

Question: user names and dots #18

Closed jsiegers closed 4 years ago

jsiegers commented 5 years ago

I noticed while trying a recent version that the script says that usernames with dots are not allowed and therefor are being replaced with a comma.

In our current setup we have a lot of users with a dot in the name all working fine. The log says the Gitlab doesn't allow this. Is this since version 12? Or did I miss something here?

Adambean commented 5 years ago

I don't recall doing this deliberately, in which case it'll be an issue for Gitlab instead. A sure way to test this would be to just try and register as a normal user with a dot in.

After some searching it appears usernames can only contain alphanumeric characters by design, so this tool won't be able to change that. What may work is attempting to sign in as that user in advance to see what user name it creates. The tool could then be adjusted to filter out characters but nothing else.

yasenv-code commented 4 years ago

Hello Adam, I have the same issue:

[warning] Dry run enabled: No changes will be persisted.
[notice] Loading configuration.
[notice] Loaded configuration.
[notice] Validating configuration.
[notice] Validated configuration.
[notice] Retrieving directory users and groups.
[notice] Establishing LDAP connection.
[debug] LDAP: Enabling debug mode
[debug] LDAP: Connecting
[debug] LDAP: Setting options
[debug] LDAP: Binding
[notice] LDAP connection established.
ldap_build_search_req ATTRS: cn cn displayName mail
[notice] 101 directory user(s) found.
[warning] User #1 [cn=y.vladimirov,ou=People,dc=awd-group,dc=tech]: Username "y.vladimirov" is incompatible with Gitlab, changed to "y,vladimirov".

And y.vladimirov is totaly accepted by the Gitlab with DOT. What we can do to fix it.

Adambean commented 4 years ago

Hi @yasenv-code,

The issue is that Gitlab's API does not (or did not, it's been a while) accept non-alphanumeric usernames when creating a user via their exposed API function. That limitation applies to any external tool.

It would appear that usernames with dots in at all was itself an afterthought: https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/7500

As this has been approximately 9 months the current Gitlab API may have resolved this by now. I don't have a sandbox I can try this on, but you're welcome to try it out yourself. What you'd need to do is checkout the current master, open "src/LdapSyncCommand.php" with your favourite text editor, then find this block at line 707:

// Make sure the username format is compatible with Gitlab later on
if (($ldapUserNameSlugified = $slugifyLdapUsername->slugify($ldapUserName)) !== $ldapUserName) {
    $this->logger->warning(sprintf("User #%d [%s]: Username \"%s\" is incompatible with Gitlab, changed to \"%s\".", $n, $ldapUserDn, $ldapUserName, $ldapUserNameSlugified));
    $ldapUserName = $ldapUserNameSlugified;
}

Simply comment out all of that as follows:

// Make sure the username format is compatible with Gitlab later on
/*
if (($ldapUserNameSlugified = $slugifyLdapUsername->slugify($ldapUserName)) !== $ldapUserName) {
    $this->logger->warning(sprintf("User #%d [%s]: Username \"%s\" is incompatible with Gitlab, changed to \"%s\".", $n, $ldapUserDn, $ldapUserName, $ldapUserNameSlugified));
    $ldapUserName = $ldapUserNameSlugified;
}
 */

When you run the sync command usernames will no longer be "slugified" at all.

Adambean commented 4 years ago

Actually, on second thoughts it could be the Slugify component, specifically the regular expression involved. What you could try instead is changing that on line 586 from

"regexp"        => "/([^A-Za-z0-9]|-_\.)+/",

to

"regexp"        => "/([^A-Za-z0-9-_\.])+/",
yasenv-code commented 4 years ago

Hello Adam, the regexp fix did the job. Now It imports all the players. I am stuck with another issue. But will open separate ticket.

Adambean commented 4 years ago

Thanks for the feedback @yasenv-code, I'll commit that shortly.

@jsiegers you may be interested in this.