tykeal / ep_ldapauth

(Up for adoption) LDAP authentication plugin for Etherpad-lite
GNU General Public License v2.0
25 stars 20 forks source link

Add "is_admin" option in ldapauth so we can use ldap roles / group #26

Open kadogo opened 9 years ago

kadogo commented 9 years ago

Hi,

Add "is_admin" option in ldapauth so we can use ldap roles / group to design who is an administator or an simple user.

Thanks.

froiloc commented 9 years ago

This is already part of the code.

Everbody who can authenticate correctly is a user. And if this user also is member of the group specified in the attribute "groupSearchBase", then that user is also an administrator and has access to /admin.

Please correct me if I got something wrong. :)

kadogo commented 9 years ago

It was me but not good account (sorry). I will look it now and say what after ^^

Edit: groupSearchBase is not the base where are the groups / roles like ou=roles,dc=domain,dc=com ?

kadogo commented 9 years ago

Ha, I found my problem again ^^ For is_admin work but we can not define who of the ldap is only a user.

Exemple: role etherpad-users: user a role etherpad-admins: user b role other application: user c

How can I auth etherpad only for user a and user b ?

tykeal commented 9 years ago

The module, as written, expects that anyone that authenticates cleanly (via the account pattern) is a valid user. This was done because when the module was written there was no easy way to outright reject a user.

The group searching is, as @froiloc states, for admin elevation.

I honestly haven't touched the code base in two years because it's been extremely low priority to me given that it's extremely under utilized where I work mostly due to my community deciding that email, IRC and wiki is preferable.

If you want the above scenario, then user c must fall outside of your account pattern.

kadogo commented 9 years ago

The users are very same, the difference is in the role that contains roleOccupant and application.

It's not possible to "force" group check for users ? So it can maybe use 2 ldapauth one for users only and a second for the admins.

froiloc commented 9 years ago

Hi @kadogo,

couldn't you use the attribute "accountPattern"?

like "(?(sAMAccountname={{username}})(objectClass=*)(memberOf='CN=etherpad-users,OU=groups,DN=domain,DN=com'))"

something like that should work. Just a quick idea. Haven't tested it.

kadogo commented 9 years ago

It should but with roleOccupant, I nor have informations about groups in the user. I will provide a ldapdump in the evening (it would be easier).

kadogo commented 9 years ago

Sorry I'm late My users doesn't have informations of the group their are member. . My roles look so: https://zerobin.net/?a99fa504180bef63#Z7O0wRNygSgJfCQmimts+w6+896ieewUOCPGg8IsrvE=

froiloc commented 9 years ago

Hi sorry for the late reply. Me weekend was busy and I had no computer with me.

If I get you correct then for each roleOccupant you have one entry. Shouldn't there be an entry in the node of the user too? Like it is with groups with the "memberOf"-attribute?

Then you could search for that pattern. I don't use roles in the ldap/ad I'm binding to.

But my guess is that if you have no entry in the user's node then you cannot filter properly.

kadogo commented 9 years ago

Bingo, I not have entry in user's node. So I thought that the easier solution is to have 2 ldapauth. One that authentificate the users and another for the admin with the flag is_admin (example).

froiloc commented 9 years ago

I'm afraid I cannot help here. My knowledge about roles in LDAP are to limited.

tykeal commented 9 years ago

@froiloc unless @kadogo is using AD and groups in AD for their etherpad access they likely don't get the memberOf reverse group attribute auto-populated. For instance, I'm backed by an OpenLDAP and adding someone to a group doesn't automatically do the reverse mapping on the user object. That's one of the (very few in my opinion) nice things that AD gives. FreeIPA might do it as well, but I don't manage anything out of it (though others on my team do).

In any case, I'm open to patches to make this work. I'm honestly not likely to do the work myself as I honestly don't care that much which is why all the changes that have gone into the code since 2013 have been PRs.

kadogo commented 9 years ago

I just did a push request.

@tykeal See if it seem good for you so we can close this issue ^^

Bantscho commented 8 years ago

I just install the following packages.

apt-get install build-essential g++