developer.jelix.org is not used any more and exists only for history. Post new tickets on the Github account.
developer.jelix.org n'est plus utilisée, et existe uniquement pour son historique. Postez les nouveaux tickets sur le compte github.

Opened 12 years ago

Closed 10 years ago

#787 closed new feature (fixed)

authentication drivers creation - driver AD (Active Directory)

Reported by: samtahRam Owned by:
Priority: normal Milestone: Jelix 1.2 beta
Component: jelix:auth Version: 1.0RC1
Severity: normal Keywords:
Cc: Blocked By:
Blocking: Documentation needed: no
Hosting Provider: Php version:

Description

AD driver authenticates users through Active Directory infos. Same principe as the other drivers

Attachments (8)

lib.diff (20.9 KB) - added by samtahRam 12 years ago.
lib patch
testapp.diff (9.5 KB) - added by samtahRam 12 years ago.
testapp patch
lib.2.diff (20.0 KB) - added by samtahRam 12 years ago.
AD driver update patch lib
testapp.2.diff (8.8 KB) - added by samtahRam 12 years ago.
AD driver update patch testapp
lib.3.diff (20.6 KB) - added by samtahRam 12 years ago.
lib.4.diff (20.5 KB) - added by samtahRam 12 years ago.
lib patch re-newed
testapp.3.diff (8.9 KB) - added by samtahRam 12 years ago.
testapp patch re-newed
ldap.auth.diff (8.6 KB) - added by fatbeard 10 years ago.
Fixes missing bracket

Download all attachments as: .zip

Change History (21)

Changed 12 years ago by samtahRam

lib patch

Changed 12 years ago by samtahRam

testapp patch

comment:1 Changed 12 years ago by laurentj

  • Milestone set to Jelix 1.2

comment:2 follow-up: Changed 12 years ago by laurentj

  • review changed from review? to review-

Thank you for this patch.

First thing: I don't know very well ldap. This is why some of my following questions could be "naïve".

  • Since it uses ldap functions, could this driver be used for any ldap ? if yes, perhaps we could call it "ldap" instead of "ad" ?
  • I see that you use often "cn={$user->login},{$this->_params['searchBaseDN']}". Perhaps we could add a configuration property like userDN="cn=%login%,...", because perhaps in some other ldap, the string should not begin by cn=.. ? (however, maybe I am wrong two...)
  • verifyPassword : you bind with the login/password of the user, but in other methods, you bind with a specific login/password. Does it mean that you cannot do some things with the user login/passord ? In getUser for example, why using the specific login/password ?
  • No password encryption ? Is it specific to ldap in general or to AD ?
  • And if we can bind with the user login / password, is it necessary to store the password as properties ? AD needs it ?
  • I think you can create a protected method which fill the jAuthAD properties with ldap attributes, and call it instead of repeating these "foreach" statements in all methods.
  • comments should be in english
  • add a space before and after "="
  • in the auth.coord.ini.php file in testapp, you have a "nacl" section. I think we should delete it ;-)
  • the [ad] section should be declared in the lib/jelix/plugins/coord/auth/auth.coord.ini.php.dist file and in the corresponding template in jelix-scripts

Except my comments, the quality of this patch is good :-)

comment:3 in reply to: ↑ 2 Changed 12 years ago by samtahRam

Replying to laurentj: My answers and the update patchs

1- Since it uses ldap functions, could this driver be used for any ldap ? if yes, perhaps we could call it "ldap" instead of "ad" ?

The purpose of this driver is to authenticate users stored in Windows Server Active Directory. Then, ldap is like some of tools used to search and manage Active Directory (e.g ldp,ldifde,adfind,dsquery,...). That's the reason why we called it 'ad'.

2- I see that you use often "cn={$user-login},{$this-_params['searchBaseDN']}". Perhaps we could add a configuration property like userDN="cn=%login%,...", because perhaps in some other ldap, the string should not begin by cn=.. ? (however, maybe I am wrong two...)

"cn={$user->login},{$this->_params['searchBaseDN']}" means the complete identification of an entry in a Active Directory partition, the CN value is the entry that we want to manage (varying according the purpose). The purpose in our case is the user identification. processing the update in the Active Directory partition must be transparent and there is no means to specify the complete identification in the config file. To join our purpose, the functions saveNewUser, removeUser, updateUser, changePassword are used to update the Active Directory partition by adding, updating or deleting an entry.

3- verifyPassword : you bind with the login/password of the user, but in other methods, you bind with a specific login/password. Does it mean that you cannot do some things with the user login/passord ? In getUser for example, why using the specific login/password ?

Users are granted or not with rights (e.g read,modify,delete) in Active directory. when we bind with a specific login/password. It is simply to authenticate user. If we want to retrieve informations, the user bound must have at last a read right in the partition. That's why a specific login/password with the necessary rights must be used. Idem for the updating process.

4- No password encryption ? Is it specific to ldap in general or to AD ?

In general, it's transparent with LDAP. Password operations can be made over unsecured (nonencrypted) connection or secured connection (SSL). Setting the configuration property 'hostname' in the ini file with the prefix 'ldaps://' allow the operations on secured connection

5- And if we can bind with the user login / password, is it necessary to store the password as properties ? AD needs it ?

AD does not return any password. But, we need the password for the authentication with jAuth. In particulary, for the event onAuthCanLogin

Changed 12 years ago by samtahRam

AD driver update patch lib

Changed 12 years ago by samtahRam

AD driver update patch testapp

comment:4 follow-up: Changed 12 years ago by laurentj

The purpose of this driver is to authenticate users stored in Windows Server Active Directory. Then, ldap is like some of tools used to search and manage Active Directory (e.g ldp,ldifde,adfind,dsquery,...). That's the reason why we called it 'ad'.

I understood that. My question was: could we use this driver (even with minor changes) for any ldap server, other than WSAD, since you use ldap functions ? Again, I don't know very well ldap, nor WSAD, this is why I ask such question. If we could use your driver for any ldap, it could be very cool ;-)

there is no means to specify the complete identification in the config file

I forgot to say: the string %login% in cn=%login%,..." could be replaced dynamically by the driver. So it means that we could use the driver for an other ldap where the DN string is different. But perhaps it is useless to have this configuration option, if in any ldap, this string should always begin by "cn=the_login,...". So, is this format of the string specific to WSAD or not ?

That's why a specific login/password with the necessary rights must be used.

Ok

Password operations can be made over unsecured (nonencrypted) connection or secured connection (SSL).

Ok, but apparently you store the password in the ldap in an unsecured manner (so an other user of WSAD, if it has read right, could read all passwords of the application). And apparently, you store the password, only for jAuth, if I well understood. So, if it is not for a feature of WSAD which would require an uncrypted password property in the record of the user, you must encrypt it ! You could use jCrypt::encrypt and jCrypt::decrypt if you want (see an example in jAuth.class.php and in the auth plugin, and you can use the same encrypted key from the configuration). At least, it should have an option to says "I want to encrypt passwords" or not (like the password_crypt_function option in the db driver of jAuth...).

Your WSAD is certainly well configured, so you don't care about the encryption of password, but it is certainly not the case of all WSAD and of all ldap of the world, and I don't want Jelix to be responsible of major security holes ;-)

Note : events like onAuthCanLogin or else don't need password.

comment:5 in reply to: ↑ 4 Changed 12 years ago by samtahRam

Replying to laurentj:

In general, we can use it for any ldap server because Microsoft Active Directory use the same principe like the other LDAP server, since we use ldap functions as you say

So, is this format of the string specific to WSAD or not ?

No, in any ldap the string begin by "cn=the_login,...".

For the password operations, I have made some changes in the patch.

Changed 12 years ago by samtahRam

comment:6 Changed 12 years ago by samtahRam

All apologizes. my mistake, i've forgotten to add something in the lib patch These are both the re-newed patchs.

Changed 12 years ago by samtahRam

lib patch re-newed

Changed 12 years ago by samtahRam

testapp patch re-newed

comment:7 Changed 12 years ago by laurentj

  • review changed from review- to review+

Patch landed in the trunk. r1283

I had some difficulties to apply the patch into the trunk: there are many errors. Please, next time, provide a patch made with the latest svn trunk !

I changed some things in tests in order to pass all tests of jelix, even if we haven't a ldap server. I also just changed some few syntaxic things into the plugin.

Thanks for your patch.

comment:8 Changed 12 years ago by laurentj

  • Resolution set to fixed
  • Status changed from new to closed

comment:9 Changed 12 years ago by laurentj

  • Documentation needed set

comment:11 Changed 10 years ago by fatbeard

  • Resolution fixed deleted
  • Status changed from closed to reopened

I beg to differ concerning the CN attribute... WSAD is ONE implementation of LDAP protocol that uses CN as the user identifier, but RFC4514 specifies that LDAP servers are required to recognize CN and UID attributes in RDN's, but that they may also recognize other attributes as such.

Hence the need to refactor all "cn=" strings into a property that can be overloaded in config files. Some organizations, including the one I actually work for, use uid instead of cn for example, and I'm pretty (but not absolutely) sure that implementations like OpenLDAP allow you to define what attribute to use for unique user identification.

Attached is a patch to take the specs into account in the driver.

  • Refactors ldap_connect and ldap_set_option in _getLinkId()
  • Refactors construction of user RDN in _buildUserDn($login)
  • Allows to specify uidProperty='attributeName' in config files, but falls back to using cn if uidProperty not provided in configuration.

Changed 10 years ago by fatbeard

Fixes missing bracket

comment:12 Changed 10 years ago by fatbeard

Additionnal refactoring could be done to also allow to specify LDAP Protocol version... even though v2 is officially deprecated, there may be some servers out there still using it... What do you think ?

comment:13 Changed 10 years ago by laurentj

  • Resolution set to fixed
  • Status changed from reopened to closed

Please create a new ticket !!!!

Note: See TracTickets for help on using tickets.