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

Can't import the User from Microsoft Active Directory #3186

Closed
phillipunzen opened this issue Oct 15, 2024 · 25 comments
Closed

Can't import the User from Microsoft Active Directory #3186

phillipunzen opened this issue Oct 15, 2024 · 25 comments

Comments

@phillipunzen
Copy link

phillipunzen commented Oct 15, 2024

Describe the bug/problem

A clear and concise description of what the bug is. If you are asking for support, please check our Wiki if your question is already answered there.

To Reproduce

Steps to reproduce the behavior:

  1. i added the LDAP-Config from my Active Directory.
  2. I click on "Import Users" and i get the Error "at least one LDAP user was not found in the database"
  3. You can see the config in the screenshots

Logfile

Error3

Expected behavior

A clear and concise description of what you expected to happen.

Screenshots
Erroe1

Error2

Environment (please complete the following information):

  • OS: [Unraid OS]
  • Python version: [Docker Container]
  • Calibre-Web version: [latest]:
  • Docker container: [LinuxServer]:
  • Special Hardware: [e.g. Rasperry Pi Zero]
  • Browser: [Google Chrome]

Additional context
Add any other context about the problem here. [e.g. access via reverse proxy, database background sync, special database location]

@OzzieIsaacs
Copy link
Collaborator

I can imagine that the problem sits behind what you marked red.
Is the message occurring with all users in your database or only with certain ones?
Is there something special behind red marking? Umlauts, special characters (like ?,\ ...)?
The displayed name is the extracted username, this doesn't look to be right, what should the real username look like?

@phillipunzen
Copy link
Author

I had only covered it in red for data protection reasons. I have therefore attached my user to you as a log. In any case, there are currently only 2 users in the group, i.e. no users are currently synchronized. Only the admin user is created.

complete log: [2024-10-15 14:44:45,070] WARN {cps.admin:1661} LDAP User: CN=Unzen, Phillip,OU=LDAP-Benutzer,OU=Benutzer,DC=phillipde,DC=local Not Found

the target username is the SAMAccountname: phillip.unzen
The displayname is "Unzen, Phillip"

But the log show the DN from the user...

@phillipunzen
Copy link
Author

Hey, I've been playing around with the "Member User Filter" a bit. If I set the filter to "distingushedName=%s" here, then I get the following error message:

Could Not Parse LDAP User: CN=Unzen, Phillip,OU=LDAP-Benutzer,OU=Benutzer,DC=phillipde,DC=local
I don't know which "error message" is closer to the target to import the Active Directory User...

@phillipunzen
Copy link
Author

I find the previous Yesterday's message that the user was not found in the database can be a bit strange. The user is supposed to be created, and therefore cannot exist yet...

@joed96
Copy link

joed96 commented Oct 20, 2024

Hi @phillipunzen , It's been a few years since I set this up, so I'm not sure if this is your issue or not... In my setup which also authenticates against Active Directory, I have LDAP Member User Filter set to: (sAMAccountName=%s).

@phillipunzen
Copy link
Author

Hey @joed96!
Thanks for your response!
Can you send me your full config?

So that I can compare mine once? I just tried it again with the Samaccount name, unfortunately without success...

Error Message: "Could not parse LDAP-User:

@joed96
Copy link

joed96 commented Oct 21, 2024

Hi @phillipunzen,

Sorry, I was going back through my system and although I can authenticate existing users, it looks like I can no longer import users from LDAP - I'm getting the same error as you are. Looking at the code, off hand, I can't really see how I ever imported users successfully.

My initial read, could be wrong through... It looks like calibre-web is trying to look up the members of the group, and then look up the user details. The problem, I think, is in this block of cps/admin.py where if the member contains an '=' then it tries to "extract" the user name; but, we need the contents of the member string (the user's DN) intact in order to look up the user details.

        if '=' in user:
            # if member object field is empty take user object as filter
            if config.config_ldap_member_user_object:
                query_filter = config.config_ldap_member_user_object
            else:
                query_filter = config.config_ldap_user_object
            try:
                user_identifier = extract_user_identifier(user, query_filter)

I'll try to get some time to look more closely at this, but off hand I'm not seeing a way to get around this in config (but I'll be happy to be proven wrong :)).

@OzzieIsaacs, to try to answer your earlier question, the username displayed in Phillip's output looks right for AD, we need this string unaltered so that we can go lookup the username and/or other user details. This should look like:

$ ldapsearch ...   '(&(cn=calibreweb-user)(objectClass=group))'     member

dn: CN=calibreweb-user,OU=Roles,OU=Groups,DC=ad,DC=example,DC=com
member: CN=John Doe,OU=Users,OU=Accounts,DC=ad,DC=example,DC=com
member: CN=Jane Doe,OU=Users,OU=Accounts,DC=ad,DC=example,DC=com


#### Loop through each member in output:

$ ldapsearch ...   '(&(objectClass=user)(distinguishedName=CN=John Doe,OU=Users,OU=Accounts,DC=ad,DC=example,DC=com))'     sAMAccountName

dn: CN=John Doe,OU=Users,OU=Accounts,DC=ad,DC=example,DC=com
sAMAccountName: johndoe


$ ldapsearch ...  '(&(objectClass=user)(distinguishedName=CN=Jane Doe,OU=Users,OU=Accounts,DC=ad,DC=example,DC=com))'     sAMAccountName

dn: CN=Jane Doe,OU=Users,OU=Accounts,DC=ad,DC=example,DC=com
sAMAccountName: janedoe

@phillipunzen
Copy link
Author

Okay nice for your work!
This sounds nice and i hope you find a solution! :)

Tell me if i can help.

@joed96
Copy link

joed96 commented Oct 23, 2024

Hi,

The basic issue above is that that both @phillipunzen and I have characters in the user's name that are not catered to by the regex in cps/admin.py extract_user_data_from_field. In my case, parenthesis, in Philip's case an escaped comma. However, I'd suggest that at least with AD this extraction is not needed and the implementation might be more robust without it. My existing user doesn't have parenthesis in their name, which is why I was previously able to import successfully.

Option 1:

Simply extend the regex in extract_user_data_from_field to cater to our additional characters. This keeps the logic the same, should be low risk (though the escaped comma is a little more tricky). However, this works until the next person comes along needing another tweak to the regex (to the point above about umlauts, special characters, etc.).

Option 2 (recommended):

Allow the configuration to leave the entire DN (e.g. CN=John Doe,OU=Users,OU=Accounts,DC=ad,DC=example,DC=com) intact to be used to look up the user record. To keep the risk of being a breaking change to a minimum, I'd propose that if the LDAP Member User Object is distinguishedName then we will want the full string without extraction logic. The extraction logic in import_ldap_users would conceptually be modified as:

if 'distinguishedName' in config.config_ldap_member_user_object:
    ### Logic for AD
    user_identifier = user
    query_filter = config.config_ldap_member_user_object

elif '=' in user:
    ### as is
else:
    ### as is

In this case, the LDAP configuration for AD users would look something like:

Configuration Setting Value
LDAP Server Host Name or IP Adress ldap.ad.example.com
LDAP Server Port 636
LDAP Encryption SSL
LDAP Authentication Simple
LDAP Administrator Username CN=Calibre-Web Authentication User,OU=Authentication,OU=Accounts,DC=ad,DC=example,DC=com
LDAP Administrator Password ****************
LDAP Distinguished Name (DN) dc=ad,dc=example,dc=com
LDAP User Object Filter (sAMAccountName=%s)
LDAP Group Object Filter (&(objectclass=group)(cn=%s))
LDAP Group Name calibreweb-user
LDAP Group Members Field member
LDAP Member User Filter Detection Custom Filter
LDAP Member User Filter (distinguishedName=%s)

I'd like to recommend Option 2 as it avoids having to rely on the extraction logic, so should be more robust across a wider variety of use cases, but is trying to be a low risk change by keeping the existing logic for anyone not trying to match the user by DN.

@OzzieIsaacs , if you're happy with this conceptually, I'll be happy to submit a PR if that's helpful. Please let me know your thoughts.

@phillipunzen
Copy link
Author

Nice work @joed96!
I will be wait hopefully for an update 👍

@OzzieIsaacs OzzieIsaacs added bug and removed question labels Oct 23, 2024
@OzzieIsaacs
Copy link
Collaborator

OzzieIsaacs commented Oct 23, 2024

I understand the problem, and I can reproduce it with my test setup. My preferred option is to extend the regex (Umlauts and Unicodes are working, just the special characters are the problem). As my fear it the next one will come and in the end it turns out he uses the magic string "distinguishedName" in some other context.

I agree with you, we would need to add every special character we know. I ended up using:
r"=((?:[@ .;\´`!+\ * ~\ < \ > \ | \°\ ^ \ & \ % ${}\³?\§\²"'#\d\s\w/[]-]|\,|\)+)"

I think we need to invert the regex, matching: everything till end of string or an unescaped ",".

I need to think on it(meaning google and chatgpt need to find an answer to this)

@joed96
Copy link

joed96 commented Oct 23, 2024

Reasonable. I agree with your idea of inverting the regex, for example:

r"=(.*?)($|(?<!\\),)"

That is, non-greedily grab all characters after the = until either (a) the end of the string, or (b) the first non-escaped comma is encountered. Small set of tests below from regex101.com (the text highlighted in green is the match.group(1)):

Screenshot 2024-10-23 at 20 07 03

@OzzieIsaacs
Copy link
Collaborator

Looks good, l‘ll try to fix it till end of the week

@joed96
Copy link

joed96 commented Oct 23, 2024

Sounds great, thank you very much for your help!

@OzzieIsaacs
Copy link
Collaborator

Please try the newest nightly version

@phillipunzen
Copy link
Author

Hey @OzzieIsaacs,
i can't update the software in the container.
image

I used the linuxserver.io Image from Docker-Hub.
I have changed the channel to nightly and click on "search updates => install updates".

@OzzieIsaacs
Copy link
Collaborator

Maybe it needs some time for them to update

@phillipunzen
Copy link
Author

To update the container or the app in the container?
I try to run later a test bare metal installation and test it.

I hope the maintainer can fast update the image...

@OzzieIsaacs
Copy link
Collaborator

Don‘t ask me this docker stuff, I don‘t know

@joed96
Copy link

joed96 commented Oct 25, 2024

Hi @OzzieIsaacs,

The update resolves the ldap user import issues I was having, though of course I'll defer to @phillipunzen as it's his issue report. One thing I noticed is that I'm receiving the following on the console now, that I wasn't previously. I'm running by cloning the master branch.

[2024-10-25 12:09:50,373]  WARN {py.warnings:112} /Users/joed/cweb/calibre-web/cps/string_helper.py:22: SyntaxWarning: invalid escape sequence '\s'
  return re.sub("(^[\s\u200B-\u200D\ufeff]+)|([\s\u200B-\u200D\ufeff]+$)","", text)

It's certainly not relevant to this thread, but just mentioning in case there were any recent changes in this part of the code.

@joed96
Copy link

joed96 commented Oct 25, 2024

You can probably disregard that warning. I restarted calibre-web and the warning did not recur, and the previously build I was using had the most recent check-in on that file. Not sure what happened there.

@phillipunzen
Copy link
Author

Unfortunately, I can't get the patch updated in the container. Even restarting the container didn't help here... Unfortunately, I can't test the "update" right now...

@phillipunzen
Copy link
Author

phillipunzen commented Nov 3, 2024

I wanted to let you know that there have been 2 updates of the image in the meantime. Unfortunately still the same problem...

[2024-11-03 19:31:07,218] WARN {cps.admin:1661} LDAP User: CN=Unzen, Phillip,OU=LDAP-Benutzer,OU=Benutzer,DC=phillipde,DC=local Not Found

@joed96
Copy link

joed96 commented Nov 3, 2024 via email

@phillipunzen
Copy link
Author

phillipunzen commented Nov 18, 2024

Sorry, i have didn't see your Message @joed96!
In calibre web shows me the version: 16.11.24, 07:21

Errormessage: [2024-11-18 07:29:26,014] WARN {cps.admin:1676} LDAP User: CN=Unzen, Phillip,OU=LDAP-Benutzer,OU=Benutzer,DC=phillipde,DC=local Not Found

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants