LDAP raw user filter gets reset when in admin page #6651
Comments
|
race condition, ick hör dir trapsen |
|
@sshipway could you apply following patch and observer whether this behaviour still occurs? diff --git a/apps/user_ldap/js/settings.js b/apps/user_ldap/js/settings.js
index acf88ef..8098967 100644
--- a/apps/user_ldap/js/settings.js
+++ b/apps/user_ldap/js/settings.js
@@ -138,6 +138,105 @@ var LdapConfiguration = {
}
};
+
+// LdapFilter object.
+
+function LdapFilter(target) {
+ this.locked = true;
+ this.target = false;
+ this.mode = LdapWizard.filterModeAssisted;
+ this.lazyRunCompose = false;
+
+ if( target === 'User' ||
+ target === 'Login' ||
+ target === 'Group') {
+ this.target = target;
+ this.determineMode();
+ }
+}
+
+LdapFilter.prototype.compose = function() {
+ if(this.locked) {
+ this.lazyRunCompose = true;
+ return false;
+ }
+
+ if(this.target === 'User') {
+ action = 'getUserListFilter';
+ } else if(this.target === 'Login') {
+ action = 'getUserLoginFilter';
+ } else if(this.target === 'Group') {
+ action = 'getGroupFilter';
+ }
+
+ if(!$('#raw'+this.target+'FilterContainer').hasClass('invisible')) {
+ //Raw filter editing, i.e. user defined filter, don't compose
+ return;
+ }
+
+ param = 'action='+action+
+ '&ldap_serverconfig_chooser='+
+ encodeURIComponent($('#ldap_serverconfig_chooser').val());
+
+ filter = this;
+
+ LdapWizard.ajax(param,
+ function(result) {
+ LdapWizard.applyChanges(result);
+ if(filter.target === 'User') {
+ LdapWizard.countUsers();
+ } else if(filter.target === 'Group') {
+ LdapWizard.countGroups();
+ LdapWizard.detectGroupMemberAssoc();
+ }
+ },
+ function (result) {
+ // error handling
+ }
+ );
+}
+
+LdapFilter.prototype.determineMode = function() {
+ param = 'action=get'+encodeURIComponent(this.target)+'FilterMode'+
+ '&ldap_serverconfig_chooser='+
+ encodeURIComponent($('#ldap_serverconfig_chooser').val());
+
+ filter = this;
+ LdapWizard.ajax(param,
+ function(result) {
+ property = 'ldap' + filter.target + 'FilterMode';
+ filter.mode = result.changes[property];
+ if(filter.mode == LdapWizard.filterModeRaw
+ && $('#raw'+filter.target+'FilterContainer').hasClass('invisible')) {
+ LdapWizard['toggleRaw'+filter.target+'Filter']();
+ } else if(filter.mode == LdapWizard.filterModeAssisted
+ && !$('#raw'+filter.target+'FilterContainer').hasClass('invisible')) {
+ LdapWizard['toggleRaw'+filter.target+'Filter']();
+ }
+ filter.unlock();
+ },
+ function (result) {
+ //on error case get back to default i.e. Assisted
+ if(!$('#raw'+filter.target+'FilterContainer').hasClass('invisible')) {
+ LdapWizard['toggleRaw'+filter.target+'Filter']();
+ filter.mode = LdapWizard.filterModeAssisted;
+ }
+ filter.unlock();
+ }
+ );
+
+}
+
+LdapFilter.prototype.unlock = function() {
+ this.locked = false;
+ if(this.lazyRunCompose) {
+ this.lazyRunCompose = false;
+ this.composeFilter();
+ }
+}
+
+// end of LdapFilter object.
+
var LdapWizard = {
checkPortInfoShown: false,
saveBlacklist: {},
@@ -145,6 +244,7 @@ var LdapWizard = {
spinner: '<img class="wizSpinner" src="'+ OC.imagePath('core', 'loading.gif') +'">',
filterModeAssisted: 0,
filterModeRaw: 1,
+ userFilter: false,
ajax: function(param, fnOnSuccess, fnOnError) {
$.post(
@@ -600,7 +700,8 @@ var LdapWizard = {
initUserFilter: function() {
LdapWizard.userFilterObjectClassesHasRun = false;
LdapWizard.userFilterAvailableGroupsHasRun = false;
- LdapWizard.regardFilterMode('User');
+ LdapWizard.userFilterModeWasDetermined = false,
+ LdapWizard.userFilter = new LdapFilter('User');
LdapWizard.findObjectClasses('ldap_userfilter_objectclass', 'User');
LdapWizard.findAvailableGroups('ldap_userfilter_groups', 'Users');
},
@@ -608,7 +709,7 @@ var LdapWizard = {
postInitUserFilter: function() {
if(LdapWizard.userFilterObjectClassesHasRun
&& LdapWizard.userFilterAvailableGroupsHasRun) {
- LdapWizard.composeFilter('user');
+ LdapWizard.userFilter.compose();
LdapWizard.countUsers();
}
},
@@ -682,12 +783,14 @@ var LdapWizard = {
&& !$('#raw'+subject+'FilterContainer').hasClass('invisible')) {
LdapWizard['toggleRaw'+subject+'Filter']();
}
+ LdapWizard.userFilterModeWasDetermined = true;
},
function (result) {
//on error case get back to default i.e. Assisted
if(!$('#raw'+subject+'FilterContainer').hasClass('invisible')) {
LdapWizard['toggleRaw'+subject+'Filter']();
}
+ LdapWizard.userFilterModeWasDetermined = true;
}
);
},
@@ -713,7 +816,7 @@ var LdapWizard = {
LdapWizard._save($('#'+originalObj)[0], $.trim(values));
if(originalObj == 'ldap_userfilter_objectclass'
|| originalObj == 'ldap_userfilter_groups') {
- LdapWizard.composeFilter('user');
+ LdapWizard.userFilter.compose();
//when user filter is changed afterwards, login filter needs to
//be adjusted, too
LdapWizard.composeFilter('login');
@@ -777,7 +880,7 @@ var LdapWizard = {
LdapWizard._save({ id: modeKey }, LdapWizard.filterModeAssisted);
if(moc.indexOf('user') >= 0) {
LdapWizard.blacklistRemove('ldap_userlist_filter');
- LdapWizard.composeFilter('user');
+ LdapWizard.userFilter.compose();
} else {
LdapWizard.blacklistRemove('ldap_group_filter');
LdapWizard.composeFilter('group');It is not the full solution, but should solve overwriting of the user filter. I will complete it when I know I works this way. |
|
I’ll give it a go. The problem only pops up sometimes – since I’m doing evaluation I was making a lot of changes to the LDAP – so it will be hard to say for sure if the change fixes it, but I’ll make a number of changes and go in and out of the pages and see what happens. Since our LDAP is very big (tens of thousands of users and thousands of groups) I’ve had to add filters to restrict the groups and users loaded, plus patch the loading functions to load in chunks of 500 rather than 30 (my test subset is <500 users) as otherwise the loading works weirdly due to LDAP not returning users alphabetically. When the bug wipes out the ldap filter, it causes everything to hang as it attempts to load the entire userbase or list of groups in one go and times out; the only fix is to add the filters to the database table manually and restart. I’ll let you know after I’ve installed it and run some tests. Steve Steve Shipway ITS Unix Services Design Lead University of Auckland, New Zealand Floor 1, 58 Symonds Street, Auckland Phone: +64 (0)9 3737599 ext 86487 DDI: +64 (0)9 923 6487 Mobile: +64 (0)21 753 189 Email: mailto:s.shipway@auckland.ac.nz s.shipway@auckland.ac.nz P Please consider the environment before printing this e-mail : 打印本邮件，将减少一棵树存活的机会 From: blizzz [mailto:notifications@github.com] @sshipway https://github.com/sshipway could you apply following patch and observer whether this behaviour still occurs? diff --git a/apps/user_ldap/js/settings.js b/apps/user_ldap/js/settings.js +// LdapFilter object.
+LdapFilter.prototype.compose = function() {
+LdapFilter.prototype.determineMode = function() {
+}
+// end of LdapFilter object.
ajax: function(param, fnOnSuccess, fnOnError) {
It is not the full solution, but should solve overwriting of the user filter. I will complete it when I know I works this way. — |
 +')
|
@sshipway after some days have passed, how does it look like? |
|
After the patch, I have not experienced the problem again, so it may well have fixed it. Of course, the problem only appears when the LDAP settings are being viewed or updated, which is not a frequent occurrence, so I suppose it may still be there. It’s a difficult thing to test definitively. Thanks, Steve Steve Shipway |
|
OK, thank you. Good enough for me, I will follow it as it will also improve the code. |
|
I can cautiously support your analysis. I have done exactly the same upgrade on a second host with exactly the same versions (synology hardware and DSM version, owncloud version from 5.0.14a to 6.0.1). On one instance everything works fine, on the other one i have a problem. The only difference I can see is that on one machine i looked into the LDAP settings (but without changing them), on the other not. Now the question is: how can I look into the working one to see what I have configured without breaking that one too? ;-) |
|
Ok, I configured it again. For the record of those who need to do it on a synology diskstation Setup the LDAP interface in: For synology DSM LDAP server select the following (wait until selection becomes accessible and then tick the entries in the dropdown lists): The following is needed, if the certificate is self-signed (no, you should not do this, get a decent certificate): To make groups work: |
|
I have make 2 test.
You have a script to delete all config LDAP in the database? I think that there is same error in config table
|
|
We were experiencing the same issue with the login filter field. I could also repeat the issue with the user filter field, but after applying the patch (with a couple of minor alterations), the user filter field was no longer being reset. I have made a pull request (#7469) that incorporates @blizz's patch with a few more alterations so that it works for all of the filter fields (user, login, and group). |
|
@ben-denham the patch is part of the story, I am working on a more complete solution, but was distracted recently unfortunately. But the smaller is maybe the better approach for the stable6-series. I'll have a look on your PR. |
…to stop filter settings from being reset under a race condition.
|
Thanks to @ben-denham for the short-cut, those who are already plagued by this bug can try the solution in #7469 |
Added improved version of patch by @blizzz in #6651 (comment) to stop filter settings from being reset under a race condition. Moved LdapFilter into a separate js file in user_ldap. Changed conditions in user_ldap's ldapFilter.js to use ===, fixed indentation. fix comparison in determineMode, fixes problems with restoring the filter mode (assisted or manually) on page refresh Give hint when composing filter failed fixing some JSHint warnings
|
Fixed with #8164 |
…ment) to stop filter settings from being reset under a race condition.
Occasionally, the LDAP raw user filter setting is changed back to the default when viewing the ldap settings in the Admin page. Since we have a restrictive filter it results in ldap no longer working correctly until I fix it in the database.
I am not able to replicate this at will, but it has occurred three times since upgrading to OC6 today.
The text was updated successfully, but these errors were encountered: