bayang / jelu

Self hosted read and to-read list book tracker
MIT License
331 stars 13 forks source link

Support LDAP bind credentials #74

Closed skiingwiz closed 1 year ago

skiingwiz commented 1 year ago

This allows configuring credentials for connecting to an LDAP server for authentication. Tested locally using the docker image (plus these changes) .

Please let me know if there are some rules for contributions that I should follow (e.g. opening an issue first). I looked but didn't see any guidelines.

bayang commented 1 year ago

Hi, thanks for your contribution. I remember testing the actual implementation against an actual ldap server but I can't remember how user and password were passed. I'm not familiar with LDAP, can you explain me why the change is required ? Do all ldap authentications need to provide this or juste some use cases ?

I aleady had a bit of sample config in the application-dev.yml config file :

  auth:
    ldap:
      enabled: false
#      url: "ldap://127.0.0.1:389/dc=home,dc=lab"
#      userDnPatterns:
#        - "uid={0}"
#        - "cn={0},ou=users"
#      userSearchFilter: "(cn={0})"

Is userDnPattern distinct from providing directly the userDn ?

Did you try to login with an actual ldap server and with those changes ?

Thanks !

skiingwiz commented 1 year ago

Some LDAP servers allow anonymous read access. Some (like mine) do not. I am not very familiar with Spring LDAP, but it looks to me like you can only connect anonymously without the changes in this pull request. With these changes, I was able to connect to my LDAP server and use it to log in. (after I added the userDn and password in the config.yml)

I believe userDn (added here) and userDnPatterns are distinct. userDn is the distinguished name of the user used to log into the LDAP server (if required), supplied to DefaultSpringSecurityContextSource. userDnPattern is supplied to AbstractLdapAuthenticator and appears to be used to turn a username in Jelu into a distinguished name in the LDAP server.

I named userDn as I did because that's what the underlying spring class calls it. I'd be happy to name that something else if you think it causes confusion.

With these changes, my working config looks very similar to the example in application-dev.yml except of course that I've added a userDn and password.

bayang commented 1 year ago

Ok everything is fine in my opinion. I just wanted to understand the nature of the change.

I can't test right now so if you tell me it's ok, then it's ok.

I suppose you have seen it in the code, but if you want your ldap user to be logged as an admin jelu user you have to add the "jelu-admin" group in the memberof attribute (in JeluLdapUserDetailsContextMapper).

Thank you for your contribution ! Hope you'll find Jelu useful.

bayang commented 1 year ago

:tada: This PR is included in version 0.40.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

skiingwiz commented 1 year ago

I pulled the new docker image and it's all working great! Thanks for the quick merge. I've been looking for something like Jelu for a while. This seems like it fits my needs exactly.