Port OrphanProcessor to D8 [#2841620]

Problem/Motivation

The OrphanProcessor does not work atm.

Proposed resolution

Lets port it as is to D8 and improve it later.

Remaining tasks

User interface changes

API changes

Data model changes

Comment	File	Size	Author
#2	2841620-1.patch	7.72 KB	webflo
#2	8.x-3.x: PHP 5.5 & MySQL 5.5, D8.3 6 pass PHP 5.5 & PostgreSQL 9.1, D8.3 6 pass PHP 5.5 & SQLite 3.8, D8.3 6 pass PHP 5.6 & MySQL 5.5, D8.3 6 pass PHP 7 & MySQL 5.5, D8.3 6 pass PHP 7 & PostgreSQL 9.1, D8.3 6 pass PHP 7 & SQLite 3.14, D8.3 6 pass

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

6 January 2017 at 15:03

webflo created an issue. See original summary.

Comment #2

webflo CreditAttribution: webflo at UEBERBIT GmbH commented 6 January 2017 at 15:04

Status:

Active

» Needs review

File	Size
2841620-1.patch	7.72 KB
8.x-3.x: PHP 5.5 & MySQL 5.5, D8.3 6 pass PHP 5.5 & PostgreSQL 9.1, D8.3 6 pass PHP 5.5 & SQLite 3.8, D8.3 6 pass PHP 5.6 & MySQL 5.5, D8.3 6 pass PHP 7 & MySQL 5.5, D8.3 6 pass PHP 7 & PostgreSQL 9.1, D8.3 6 pass PHP 7 & SQLite 3.14, D8.3 6 pass

Comment #3

webflo CreditAttribution: webflo at UEBERBIT GmbH commented 6 January 2017 at 15:06

+++ b/ldap_servers/src/OrphanProcessor.php
@@ -43,40 +44,39 @@ class OrphanProcessor {
-      \Drupal::configFactory()->getEditable('ldap_user.settings')->set('ldap_user_cron_last_uid_checked', 1)->save();

I moved the ldap_user_cron_last_uid_checked to the state system. Lets add an update hook to delete the old setting.

Comment #4

webflo CreditAttribution: webflo at UEBERBIT GmbH commented 6 January 2017 at 15:41

+++ b/ldap_servers/src/OrphanProcessor.php
@@ -154,34 +154,28 @@ class OrphanProcessor {
+          $account = User::load($user_data['uid']);
+          $account->ldap_user_last_checked->value = $check_time;
+          $account->save();

I think we shouldn't touch all user objects because this could lead to a lot of cache invalidation. The ldap_user_last_checked time it not used for the query and therefore not relevant.

Comment #5

grahl

he/him

Zürich

CreditAttribution: grahl as a volunteer commented 7 January 2017 at 18:15

Assigned:

Unassigned

» grahl

Comment #6

7 January 2017 at 19:16

grahl committed 230cf26 on 8.x-3.x authored by webflo

Issue #2841620 by webflo, grahl: Port OrphanProcessor to D8

Comment #7

grahl

he/him

Zürich

CreditAttribution: grahl as a volunteer commented 7 January 2017 at 19:28

Status:

Needs review

» Fixed

Thanks for starting to get ball rolling on this webflo!

I've finished porting the orphan processor and the mail functionality seems to work fine now, though I'm not convinced that this actually works properly on 7, since I've had to modify searchAllBaseDN slightly: The puid is often representative of the whole DN, a search across that within the base DN will in most cases likely return 0 results, thus this has to be stripped out. At least for the Docker test configuration this is the case.

There could be real-world configurations where that is not the case. If one of those turns up I'll add it as a new use case to testSearchAllBaseDns() but I'd require some feedback here.

I agree that in the previous configuration saving accounts was not ideal when ldap_user_last_checked was not taken into account. I've now added a field to the configuration which allows one to check daily, monthly, weekly or always thus this should now server an actual purpose. I'm open to suggestions if someone had a better solution.

Moving ldap_user_cron_last_uid_checked to state is a good solution, since we are still in alpha releases and the setting doesn't really hurt I'm not writing an update hook for that. If you think we really need that I'd accept another patch ;-)

I have not merged the first part of your patch modifying userPuidFromLdapEntry() since that's unrelated. Could you please add a separate ticket for that? I'd like to not add that without exactly knowing what side-effects this could have.