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

[v1.0.0b] - LDAP login/user creation not possible due to an incorrect if statement in the code #1775

Closed
5 tasks done
ERLKDev opened this issue Oct 26, 2022 · 4 comments
Closed
5 tasks done

Comments

@ERLKDev
Copy link
Contributor

ERLKDev commented Oct 26, 2022

First Check

  • This is not a feature request
  • I added a very descriptive title to this issue.
  • I used the GitHub search to find a similar issue and didn't find it.
  • I searched the Mealie documentation, with the integrated search.
  • I already read the docs and didn't find an answer.

What is the issue you are experiencing?

When using LDAP, it is not possible to login or create an user due to an incorrect if statement.

In the security.py code, on line 83, a search for the user dn and attributes is done within LDAP, which returns an array with the matches. Afterward, an if statement check the result of the search query. However, this if statement has an incorrect negation in it.

The current logic checks if a results is found, if that is the case, false will be returned which equals an incorrect login. The not should be removed from this if statement.

Current code:

    # Search "username" against "cn" attribute for Linux, "sAMAccountName" attribute
    # for Windows and "mail" attribute for email addresses. The "mail" attribute is
    # required to obtain the user's DN for the LDAP_ADMIN_FILTER.
    user_entry = conn.search_s(
        settings.LDAP_BASE_DN,
        ldap.SCOPE_SUBTREE,
        f"(&(objectClass=user)(|(cn={username})(sAMAccountName={username})(mail={username})))",
        ["name", "mail"],
    )
    if not user_entry:
        user_dn, user_attr = user_entry[0]
    else:
        return False

The code should look like this:

    # Search "username" against "cn" attribute for Linux, "sAMAccountName" attribute
    # for Windows and "mail" attribute for email addresses. The "mail" attribute is
    # required to obtain the user's DN for the LDAP_ADMIN_FILTER.
    user_entry = conn.search_s(
        settings.LDAP_BASE_DN,
        ldap.SCOPE_SUBTREE,
        f"(&(objectClass=user)(|(cn={username})(sAMAccountName={username})(mail={username})))",
        ["name", "mail"],
    )
    if user_entry:
        user_dn, user_attr = user_entry[0]
    else:
        return False

After the above fix, login is possible.

However, if a user is created, the user has the binary values within the name and email fields, instead of the string values.

The following code creates the user.

        user = db.users.create(
            {
                "username": username,
                "password": "LDAP",
                "full_name": user_attr["name"][0],
                "email": user_attr["mail"][0],
                "admin": False,
            },
        )

The problem is the user_attr values are binary instead of strings. When looking at the python-ldap documentation this is to be expected. In the documentation it states the following:
Attribute values, on the other hand, MAY contain any type of data, including text. To know what type of data is represented, python-ldap would need access to the schema, which is not always available (nor always correct). Thus, attribute values are always treated as bytes. Encoding/decoding to other formats – text, images, etc. – is left to the caller.

This means the user attributes (name and mail) need to be decoded to string.

Deployment

Docker (Linux)

Deployment Details

Docker install using a portainer stack on an unraid system.

@stale
Copy link

stale bot commented Dec 26, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Dec 26, 2022
@tomamplius
Copy link
Contributor

@ERLKDev can you check with last version

@pwood
Copy link

pwood commented Jan 3, 2023

@tomamplius Not OP - but your PR fixes the initial logic issue that prevented the login.

However the issue with name and email encoding is still present, for example LDAP users imported from Authentik's LDAP proxy:

Name: \x506574657220412e20576f6f64
Email: \x706574657240616c6173747269612e6e6574

Which should be Peter Wood and peter@alastria.net.

@cmintey
Copy link
Contributor

cmintey commented Apr 5, 2023

@hay-kot this issue can be closed. It should have been closed with the initial LDAP fix PR, but I missed it

@hay-kot hay-kot closed this as completed Apr 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants