Description: ------------ Complex updates that change the objectClasses fail because the modifications are split out into add/del/modify. Because most classes have some 'must' attributes these need to be added but update tries to add those first before modifying the objectClass attribute. Some for removing an objectClass and all related attributes. This can be solved by creating an array that is passed to ldap_modify that does everything at once. I solved it by doing this: // $account is my Net_LDAP2_Entry $changes = $account->getChanges(); $values = array_merge($changes['replace'], $changes['delete'], $changes['add']); echo ldap_modify($account->getLDAP()->getLink(), $account->dn(), $values); Removing attributes is possible by setting the attribute to array()

Thank you for reporting this bug. The problem is, like you described, that the modification order "add", "delete", "replace" is fixed in update(). The modifications are currently not carried out in the same order they are called on the object. Please try the following code: ---------snip-------- // $account is your Net_LDAP2_Entry // $ldap is your Net_LDAP2 object $changes = $account->getChanges(); $res = $ldap->modify($account, array('changes' => 'replace' => $changes['replace'], 'delete' => $changes['delete'], 'add' => $changes['add'])); if ($res instanceof Net_LDAP2_Error) { echo "UPDATE FAILED: ".$res->getMessage(); } else { echo "UPDATE OK"; } ---------snap-------- to see if that solves the problem. If it does, then we must introduce some code inside the update() method that first applies changes to the objectclass attribute.

I've tried something like that. The main problem is that ldap doesn't support transactions so you need to go from valid to valid states. This means that all changes (attributes) that are required for changing (adding, replacing or deleting) an objectclass from a entry need to be in one request. For this the ldap_modify function is needed. I'm not sure but I think the update method can be rewritten to create just one change that can be send to ldap_modify. This would also be quite a bit faster because it only results in one call to the ldap server.

Hm, a short workaround (which is the current expected behavior since the library does not know anything about user code) would be to split the modifications that affect objectclasses inside the usercode; e.g.: ---------snip------- // Case 1: Add new OCL $entry->replace(array('objectClass' => array('oldOCL', 'newOCL'))); $entry->update(); $entry->add(....); // Case2: Delete OCL $entry->delete('attribute1_from_OCL_TBDEL', 'attribute2_from_OCL_TBDEL'); $entry->update(); $entry->delete('objectClass' => 'OCL_TBDEL'); $entry->update(); ------------snap----------- instead of doing it all at once. For a long term solution i see two fixes: 1) adjust update() to only use one ldap_modify() call (does that work in every situation?) 2) adjust update() to carry out modifications to objectclass attribute prior adjusting the rest of the attributes. In case of a deletion of an objectclass from the list, all affected filled attributes must be cleared out. However, with both it must considered that the local copy may not reflect the servers state since the user might not select every attribute that is flagged "must". So because this lack of knowledge, in such situations the update() method needs to refetch the attributes of the to be deleted object class (which are not also provided by other OCLs!) to be able to make the neccessary decision which attributes to delete. This results in performance overhead. Maybe i am just thinking too complex, do you have any other idea?

I would favor option 1 if it works safely in every situation. would you like to try to rewrite that part of update()?

I think the only way to go is 1, because when schemachecks are on (which seems the default on rhel 5) you get errors from ldap if you don't do all changes at once. I got the use of ldap_modify from phpldapadmin. I'll try to create a patch in the next few days but I won't have a lot of time. I'll keep you posted.

Something new on that topic? Did you have time to research some patches?

Unfortunately this is not easy to fix due to Net_LDAP's architecture. Net_LDAP was meant to modify parts of entries without the need to know the entire entry in the server. I tried to rewrite update() so it does everything with just one LDAP-query but this is not easily possible. Maybe you will find a good solution to this, but a good workaround is to remove the entry and add it with the new objectclass. This way, you also can add structual objectclass changes. I will document that and close the bug.

I haven't got the time to look at this because of my new job. Deleting is just to dangerous.

I have started to look into this issue as our LDAP directory has strict permissions, preventing the deletion workaround from working. This issue is interlinked with a number of other issues: - If you try to delete a non-retrieved attribute when using LDAPv3 it will fail - If you try to add a value to a non-retrieved attribute then all other values will be erased To fix both this issue and the two above, I intend to make it automatically retrieve the entry if the attribute being changed is *not* loaded.

I have done some research into the background of this problem, and it turns out that the cause of this issue lies directly with the php-ldap api itself. http://www.mail-archive.com/internals@lists.php.net/msg11100.html Apparently php's ldap_modify function is actually ldap_modify_replace in OpenLDAP's API for unknown reasons. Would it be better to get this fixed in php-ldap or to implement a mild hack and just retrieve the full entry and build the needed changes array from that?

Added patch change-entry-use-ldap-modify-once.

Added patch use-ldap-modify-once.

I've attached a patch which does two things: 1) Moves it to using a single ldap_modify() command for all modifications 2) Disables the deletion of the old RDN attribute I needed both as otherwise my changes were rejected with LDAP_OBJECT_CLASS_VIOLATION (as the old RDN is a MUST attribute of a Object Class) when attempting to perform moves.

The problem with retrievability is, that some attributes ma ynever be retrievable at all, they are only replaceable (like password fields). There should be some mechanism that retrieving the attribute is only tried once, and also an option to disable that feature alltogether since it slows down the library considerably under some circumstances. I reopened that bug for further discussion.

Yes, I considered that issue. I am still working on the automatic retrieval - I intend to use a private boolean which informs it if it needs to retrieve the full set or not. So the retrieval should only happen once. Would a property held by the server to control if autoretrieval should be attempted be the best place to put the "global" switch to disable this behaviour? The patch I have already attached should work for the replace case, except for Microsoft AD (which I hear requires a simultaneous delete/add which PHP cannot do due to weaknesses in it's LDAP api)

Well im not aware of any information on a standard LDAP server that lets us decide on the autoretrieval. Retrievability is part of the servers ACL and this is usually not disclosured to the clients for security reasons. The only place where i would put that would be Net_LDAPs config.

Another thing to consider is, that not all entrys represent actual ldap entries. Sometimes people build up dummy entries to just carry out a specific operation (like replacing passwords or adding values regardless of the ldap state). So the attribute retrieval must be a global default option, but also may be overriden at the entry-object level to be able to turn it off for each entry individually. A newly created entry would pull the config from the ldap object when it becomes necessary (e.g. it gets linked to the ldap object) By standard i would turn off this autofetch feature.

Property held by the server = Net::LDAP2 class configuration. Auto-retrieval would of course only be enabled if permitted by the Net::LDAP2 configuration - and it has a LDAP Entry it is representing. Can you please examine the patch I have uploaded at the moment? It alters the update() function to use ldap_modify() - building the list of changes using a freshly retrieved copy of the entry which has all attributes requested.

The patch (use-ldap-modify-once) looks good on a first glance. Can you confirm the unit tests are running well with it applied?

As far as I can tell - at least on my system with the unit tests - they run the same regardless of the patch being applied or not. Results = Tests: 94, Assertions: 406, Failures: 2, Incomplete: 31, Skipped: 34 Failures were in Net_LDAP2_LDIFTest and Net_LDAP2_FilterTest which were untouched by my patch. Not sure about the 31 incomplete tests though.

Beni, what is the status of this being committed? Do I need to adjust my test configuration somehow do provide correct results?

Hi, i don't think that you need to adjust something. The tests look good so far, however i didn't had the time to verify and integrate it into the trunk. Expect the patch to be applied. If you have the time, it would be very cool if you could write some unit tests for modify.

Added patch improved-ldap-modify-once.

I notice this has not yet been committed yet. Please find attached a updated version. The current version will fail to delete attributes under certain circumstances, as it does not rekey the values. It also handles the case where you are adding and deleting attributes of the same name at the same time.

I did some small tests and applied the fix into SVN as the code looks good so far. If i get some spare time, i will test it deeper and write some unit tests. Until this is done, i defer the next release; hopefully i get time during the week. The bug stays open until i can confirm the tests. Thank you for your huge effort!

The patch has been release with 2.1.0 already, so I just close this.