-
-
Notifications
You must be signed in to change notification settings - Fork 728
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
add LDAP auth support #296
Comments
I like the idea of integrating with LDAP or other auth systems, but when we discussed in #19 (please read!), there was no clear winner of technologies. Everybody uses different things and wants different things. If I wanted to please everyone I'd do nothing but integrate auth systems. For this precise reason, I hate dealing with auth. You put a ton of work into it and in the end, you make 1/5 people happy, and the others just say "why didn't you do X instead". So I won't be doing this in the near future, but I'm happy to accept pull requests (provided the design has been discussed with me first). 👍 |
Thanks for sharing the link. I don't think it is good to have n number of auth providers although some standard ones could be ok. You could use OIDC for auth on most of the servers. Here are links to some of the popular website configurations. (You will need to view source or use curl for some of these as browsers may show blank screen while rendering json).
Here are some reasons why I chose LDAP:
There are two distinct pieces that should be tackled separately - Authentication and ACLS. (not familiar with go so adding psuedo code to exiting code). func (a *SQLiteAuth) Authenticate(username, password string) (*User, error) {
if username == Everyone {
return nil, ErrUnauthenticated
}
user, err := a.User(username)
if ldapEnabled {
if user == nil {
if allowLdapNewUserSync {
user, err := a.AddLdapUser(username, password)
}
} else user.account_type == 'ldap' && user.synced_time > ago(1h) {
user, err := a.SyncLdapUser(username, password)
}
}
if err != nil {
bcrypt.CompareHashAndPassword([]byte(intentionalSlowDownHash),
[]byte("intentional slow-down to avoid timing attacks"))
return nil, ErrUnauthenticated
}
if err := bcrypt.CompareHashAndPassword([]byte(user.Hash), []byte(password)); err != nil {
return nil, ErrUnauthenticated
}
return user, nil
}
func (a *SQLiteAuth) AddLdapUser(username, password) {
var valid, err := AuthenticateViaLdap(username, password)
if valid {
a.AddUser(username, password)
a.SyncLdapGroups(username)
}
}
func (a *SQLiteAuth) SyncLdapUser(username, password) {
var valid, err := AuthenticateViaLdap(username, password)
if valid {
// update user.synced_time
a.SyncLdapGroups(username)
} else {
// remove user and return nil
}
} I wouldn't worry about groups now as this could be manually added by the cli. But having at least While this allows on-demand sync and could work for large number of ldap user, it does sprinkle the code. The other option would be to have a background job that syncs with an ldap server every X mins and update the sqlite table. But need to verify if servers return password hash. |
I don't think I want to add LDAP support right now, but that doesn't mean it'll forever stay this way. I've got my hands full with the iOS app and the 50 other tickets, but I'll leave this open for a while. You are of course free to fork and add LDAP support to your fork. If it's good code, I'll definitely consider merging it. |
I was reading through #812 and came across a message of @wunter8
I think that LDAP would be a great addition here to possibly create the user inside NTFY when it doesn't exist, preventing errors like this. What is the opinion about this approach? |
I would like to use LDAP auth for all my apps that I host as I can centrally manage users/passwords and groups for my family. Changing password in one place can change password everywhere. This also allows me to enforce certain password policy centrally.
I have found authelia's configuration to be the best that allows granular configuration. https://www.authelia.com/docs/configuration/authentication/ldap.html
This should be tackled in multiple steps.
The text was updated successfully, but these errors were encountered: