From ac8632c63feb95fcbae6483a2245f57b45ecf0d1 Mon Sep 17 00:00:00 2001 From: Asko Soukka Date: Tue, 18 Oct 2016 15:57:31 +0300 Subject: [PATCH 01/17] Replace mutable interfaces with read-only ones --- src/pas/plugins/ldap/plugin.py | 8 +------- src/pas/plugins/ldap/sheet.py | 4 ++-- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/src/pas/plugins/ldap/plugin.py b/src/pas/plugins/ldap/plugin.py index ea4f516..fec6d7c 100644 --- a/src/pas/plugins/ldap/plugin.py +++ b/src/pas/plugins/ldap/plugin.py @@ -103,13 +103,7 @@ def _wrapper(self, *args, **kwargs): pas_interfaces.IGroupsPlugin, pas_interfaces.IPropertiesPlugin, pas_interfaces.IUserEnumerationPlugin, - plonepas_interfaces.capabilities.IDeleteCapability, - plonepas_interfaces.capabilities.IGroupCapability, - plonepas_interfaces.capabilities.IPasswordSetCapability, - plonepas_interfaces.group.IGroupManagement, - plonepas_interfaces.group.IGroupIntrospection, - plonepas_interfaces.plugins.IMutablePropertiesPlugin, - plonepas_interfaces.plugins.IUserManagement) + plonepas_interfaces.group.IGroupIntrospection) class LDAPPlugin(BasePlugin): """Glue layer for making node.ext.ldap available to PAS. """ diff --git a/src/pas/plugins/ldap/sheet.py b/src/pas/plugins/ldap/sheet.py index 22599c0..b9127c9 100644 --- a/src/pas/plugins/ldap/sheet.py +++ b/src/pas/plugins/ldap/sheet.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- from Acquisition import aq_base -from Products.PlonePAS.interfaces.propertysheets import IMutablePropertySheet from Products.PluggableAuthService.UserPropertySheet import UserPropertySheet +from Products.PluggableAuthService.interfaces.propertysheets import IPropertySheet # noqa from node.ext.ldap.interfaces import ILDAPGroupsConfig from node.ext.ldap.interfaces import ILDAPUsersConfig from zope.globalrequest import getRequest @@ -12,7 +12,7 @@ logger = logging.getLogger('pas.plugins.ldap') -@implementer(IMutablePropertySheet) +@implementer(IPropertySheet) class LDAPUserPropertySheet(UserPropertySheet): def __init__(self, principal, plugin): From 4a242b44a99695dc319b68174d18c972c60acfec Mon Sep 17 00:00:00 2001 From: Asko Soukka Date: Tue, 18 Oct 2016 15:58:03 +0300 Subject: [PATCH 02/17] Wrap PAS plugin interface with ram.cache-decorators --- src/pas/plugins/ldap/plugin.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/pas/plugins/ldap/plugin.py b/src/pas/plugins/ldap/plugin.py index fec6d7c..e7650db 100644 --- a/src/pas/plugins/ldap/plugin.py +++ b/src/pas/plugins/ldap/plugin.py @@ -20,6 +20,7 @@ from pas.plugins.ldap.interfaces import VALUE_NOT_CACHED from pas.plugins.ldap.sheet import LDAPUserPropertySheet from zope.interface import implementer +from plone.memoize import ram import ldap import logging import os @@ -96,6 +97,17 @@ def _wrapper(self, *args, **kwargs): return _decorator +def cacheKey(func, *args, **kwargs): + key = __file__ + key += ':' + func.__name__ + key += ':' + str(int(time.time() // 10)) + for arg in ([args] + list(kwargs.values())): + if isinstance(arg, unicode): + arg = arg.encode('utf-8') + key += ':' + str(arg) + return key + + @implementer( ILDAPPlugin, pas_interfaces.IAuthenticationPlugin, @@ -211,6 +223,7 @@ def authenticateCredentials(self, credentials): # Allow querying groups by ID, and searching for groups. # @security.private + @ram.cache(lambda f, o, *args, **kwargs: cacheKey(f, *args, **kwargs)) def enumerateGroups(self, id=None, exact_match=False, sort_by=None, max_results=None, **kw): """ -> ( group_info_1, ... group_info_N ) @@ -288,6 +301,7 @@ def enumerateGroups(self, id=None, exact_match=False, sort_by=None, # # Determine the groups to which a user belongs. @security.private + @ram.cache(lambda f, o, *args, **kwargs: cacheKey(f, args[0])) def getGroupsForPrincipal(self, principal, request=None): """principal -> ( group_1, ... group_N ) @@ -319,6 +333,7 @@ def getGroupsForPrincipal(self, principal, request=None): # @ldap_error_handler('enumerateUsers') @security.private + @ram.cache(lambda f, o, *args, **kwargs: cacheKey(f, *args, **kwargs)) def enumerateUsers(self, id=None, login=None, exact_match=False, sort_by=None, max_results=None, **kw): """-> ( user_info_1, ... user_info_N ) @@ -475,6 +490,7 @@ def removePrincipalFromGroup(self, principal_id, group_id): # which case the properties are not persistently mutable). # @security.private + @ram.cache(lambda f, o, *args, **kwargs: cacheKey(f, args[0])) def getPropertiesForUser(self, user_or_group, request=None): """User -> IMutablePropertySheet || {} @@ -633,6 +649,7 @@ def getGroups(self): """ return map(self.getGroupById, self.getGroupIds()) + @ram.cache(lambda f, o: cacheKey(f)) def getGroupIds(self): """ Returns a list of the available groups (ids) From 3f44d60a5b121809d4c0847a8ef8abde7fd483eb Mon Sep 17 00:00:00 2001 From: Asko Soukka Date: Tue, 18 Oct 2016 15:59:13 +0300 Subject: [PATCH 03/17] Fix getGroupsForPrincipal to call user.group_ids from node.ext.ldap for better performance --- src/pas/plugins/ldap/plugin.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/pas/plugins/ldap/plugin.py b/src/pas/plugins/ldap/plugin.py index e7650db..122d1e0 100644 --- a/src/pas/plugins/ldap/plugin.py +++ b/src/pas/plugins/ldap/plugin.py @@ -321,9 +321,7 @@ def getGroupsForPrincipal(self, principal, request=None): # return if there is no user return tuple() if self.groups: - # XXX: provide group_ids function in UGM! Way too calculation-heavy - # now - return [_.id for _ in _principal.groups] + return _principal.group_ids return tuple() # ## From 388b42d7925541638c835e9cfb7fe98f562c0894 Mon Sep 17 00:00:00 2001 From: Asko Soukka Date: Tue, 18 Oct 2016 16:00:13 +0300 Subject: [PATCH 04/17] Fix getGroupById to only get group details if group id is available for better performance --- src/pas/plugins/ldap/plugin.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/pas/plugins/ldap/plugin.py b/src/pas/plugins/ldap/plugin.py index 122d1e0..6b42b1b 100644 --- a/src/pas/plugins/ldap/plugin.py +++ b/src/pas/plugins/ldap/plugin.py @@ -613,7 +613,8 @@ def getGroupById(self, group_id): """ group_id = decode_utf8(group_id) groups = self.groups - if not groups or group_id not in groups.keys(): + groups_ids = self.getGroupIds() + if not groups or group_id not in groups_ids: return None ugmgroup = self.groups[group_id] title = ugmgroup.attrs.get('title', None) From c5093047903f6351b4f712a8feae0de0127a3cb9 Mon Sep 17 00:00:00 2001 From: Asko Soukka Date: Thu, 20 Oct 2016 12:36:51 +0300 Subject: [PATCH 05/17] Use bda.cache with pylibmc --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index c22f487..d28585e 100644 --- a/setup.py +++ b/setup.py @@ -40,7 +40,7 @@ install_requires=[ 'AccessControl>=3.0', 'Acquisition', - 'bda.cache', + 'bda.cache[pylibmc]', 'five.globalrequest', 'node', 'node.ext.ldap>=1.0b2', From 3acb4d34d787c393c06de5e46183e59aa91b6276 Mon Sep 17 00:00:00 2001 From: Asko Soukka Date: Thu, 20 Oct 2016 12:38:44 +0300 Subject: [PATCH 06/17] Optimize node.ext.ldap searches and use mapped (default) saarch attrlist to normalize searches when possible --- src/pas/plugins/ldap/plugin.py | 81 +++++++++++++++++++++++++++------- 1 file changed, 64 insertions(+), 17 deletions(-) diff --git a/src/pas/plugins/ldap/plugin.py b/src/pas/plugins/ldap/plugin.py index 6b42b1b..4ecfbde 100644 --- a/src/pas/plugins/ldap/plugin.py +++ b/src/pas/plugins/ldap/plugin.py @@ -277,12 +277,19 @@ def enumerateGroups(self, id=None, exact_match=False, sort_by=None, cookie = None while True: try: - batch_matches, cookie = groups.search( + res = groups.search( criteria=kw, + attrlist=exact_match and + groups.context.search_attrlist or None, exact_match=exact_match, - page_size=self._ldap_props.page_size, + page_size=not exact_match and + self._ldap_props.page_size or None, cookie=cookie, ) + if isinstance(res, tuple): + batch_matches, cookie = res + else: + batch_matches, cookie = res, '' except ValueError: return tuple() matches += batch_matches @@ -291,7 +298,17 @@ def enumerateGroups(self, id=None, exact_match=False, sort_by=None, if sort_by == 'id': matches = sorted(matches) pluginid = self.getId() - ret = [dict(id=encode_utf8(_id), pluginid=pluginid) for _id in matches] + ret = list() + if exact_match: + for id, attrs in matches: + ret.append({ + 'id': encode_utf8(id), + 'pluginid': pluginid}) + else: + for id in matches: + ret.append({ + 'id': encode_utf8(id), + 'pluginid': pluginid}) if max_results and len(ret) > max_results: ret = ret[:max_results] return ret @@ -310,18 +327,22 @@ def getGroupsForPrincipal(self, principal, request=None): o May assign groups based on values in the REQUEST object, if present """ - users = self.users - if not users: + groups = self.groups + if not groups: return tuple() - try: - _principal = self.users[principal.getId()] - except KeyError: + + _id = principal.getId() + + if _id in self.getGroupIds(): # getGroupIds is cached groups.ids # XXX: that's where group in group will happen, but so far # group nodes do not provide membership info so we just # return if there is no user return tuple() - if self.groups: - return _principal.group_ids + users = self.users + try: + return users and users[_id].group_ids or [] + except KeyError: + pass return tuple() # ## @@ -393,17 +414,27 @@ def enumerateUsers(self, id=None, login=None, exact_match=False, users = self.users if not users: return tuple() + if not exact_match: + for value in users.principal_attrmap.values(): + kw[value] = kw.values()[0] matches = [] cookie = None while True: try: - batch_matches, cookie = users.search( + res = users.search( criteria=kw, - attrlist=('login',), + attrlist=exact_match and + users.context.search_attrlist or ['login'], exact_match=exact_match, - page_size=self._ldap_props.page_size, + or_search=not exact_match, + page_size=not exact_match and + self._ldap_props.page_size or None, cookie=cookie, ) + if isinstance(res, tuple): + batch_matches, cookie = res + else: + batch_matches, cookie = res, '' except ValueError: return tuple() matches += batch_matches @@ -502,7 +533,7 @@ def getPropertiesForUser(self, user_or_group, request=None): """ ugid = user_or_group.getId() try: - if self.enumerateUsers(id=ugid) or self.enumerateGroups(id=ugid): + if ugid in self.getGroupIds() or self.users[ugid]: return LDAPUserPropertySheet(user_or_group, self) except KeyError: pass @@ -613,8 +644,7 @@ def getGroupById(self, group_id): """ group_id = decode_utf8(group_id) groups = self.groups - groups_ids = self.getGroupIds() - if not groups or group_id not in groups_ids: + if not groups or group_id not in self.getGroupIds(): return None ugmgroup = self.groups[group_id] title = ugmgroup.attrs.get('title', None) @@ -653,7 +683,24 @@ def getGroupIds(self): """ Returns a list of the available groups (ids) """ - return self.groups and self.groups.ids or [] + groups = self.groups + if not groups: + return [] + matches = [] + cookie = None + while True: + try: + batch_matches, cookie = groups.search( + attrlist=['id'], + page_size=self._ldap_props.page_size, + cookie=cookie, + ) + except ValueError: + return tuple() + matches += batch_matches + if not cookie: + break + return [group[1]['id'][0] for group in matches] def getGroupMembers(self, group_id): """ From 9ea76a597926fa63506855d06b01d3ab876b6b7c Mon Sep 17 00:00:00 2001 From: Asko Soukka Date: Thu, 20 Oct 2016 14:38:57 +0300 Subject: [PATCH 07/17] Fix to send only bytestrings to node.ext.ldap --- src/pas/plugins/ldap/plugin.py | 17 ++++++++--------- src/pas/plugins/ldap/properties.py | 7 +++++-- src/pas/plugins/ldap/sheet.py | 7 +++++-- 3 files changed, 18 insertions(+), 13 deletions(-) diff --git a/src/pas/plugins/ldap/plugin.py b/src/pas/plugins/ldap/plugin.py index 4ecfbde..4aa1630 100644 --- a/src/pas/plugins/ldap/plugin.py +++ b/src/pas/plugins/ldap/plugin.py @@ -9,8 +9,8 @@ from Products.PluggableAuthService.permissions import ManageGroups from Products.PluggableAuthService.permissions import ManageUsers from Products.PluggableAuthService.plugins.BasePlugin import BasePlugin -from node.ext.ldap.base import decode_utf8 -from node.ext.ldap.base import encode_utf8 +from node.utils import decode +from node.utils import encode from node.ext.ldap.interfaces import ILDAPGroupsConfig from node.ext.ldap.interfaces import ILDAPProps from node.ext.ldap.interfaces import ILDAPUsersConfig @@ -278,7 +278,7 @@ def enumerateGroups(self, id=None, exact_match=False, sort_by=None, while True: try: res = groups.search( - criteria=kw, + criteria=encode(kw), attrlist=exact_match and groups.context.search_attrlist or None, exact_match=exact_match, @@ -302,12 +302,12 @@ def enumerateGroups(self, id=None, exact_match=False, sort_by=None, if exact_match: for id, attrs in matches: ret.append({ - 'id': encode_utf8(id), + 'id': id, 'pluginid': pluginid}) else: for id in matches: ret.append({ - 'id': encode_utf8(id), + 'id': id, 'pluginid': pluginid}) if max_results and len(ret) > max_results: ret = ret[:max_results] @@ -422,7 +422,7 @@ def enumerateUsers(self, id=None, login=None, exact_match=False, while True: try: res = users.search( - criteria=kw, + criteria=encode(kw), attrlist=exact_match and users.context.search_attrlist or ['login'], exact_match=exact_match, @@ -444,7 +444,7 @@ def enumerateUsers(self, id=None, login=None, exact_match=False, ret = list() for id, attrs in matches: ret.append({ - 'id': encode_utf8(id), + 'id': id, 'login': attrs['login'][0], 'pluginid': pluginid}) if max_results and len(ret) > max_results: @@ -642,7 +642,6 @@ def getGroupById(self, group_id): Returns the portal_groupdata-ish object for a group corresponding to this id. None if group does not exist here! """ - group_id = decode_utf8(group_id) groups = self.groups if not groups or group_id not in self.getGroupIds(): return None @@ -658,7 +657,7 @@ def getGroupById(self, group_id): data = propfinder.getPropertiesForUser(group, None) if not data: continue - group.addPropertysheet(propfinder_id, data) + group.addPropertysheet(propfinder_id, decode(data)) # add subgroups group._addGroups(pas._getGroupsForPrincipal(group, None, plugins=plugins)) diff --git a/src/pas/plugins/ldap/properties.py b/src/pas/plugins/ldap/properties.py index fa2a8f3..6e3c6a8 100644 --- a/src/pas/plugins/ldap/properties.py +++ b/src/pas/plugins/ldap/properties.py @@ -174,7 +174,10 @@ def propproxy(ckey): def _getter(context): value = context.plugin.settings.get(ckey, DEFAULTS[ckey]) - return value + if isinstance(value, unicode): + return value.encode('utf-8') + else: + return value def _setter(context, value): context.plugin.settings[ckey] = value @@ -219,7 +222,7 @@ def memcached(self, value): recordProvider = queryUtility(ICacheSettingsRecordProvider) if recordProvider is not None: record = recordProvider() - record.value = value.decode('utf8') + record.value = value else: return u'feature not available' diff --git a/src/pas/plugins/ldap/sheet.py b/src/pas/plugins/ldap/sheet.py index b9127c9..7a995f8 100644 --- a/src/pas/plugins/ldap/sheet.py +++ b/src/pas/plugins/ldap/sheet.py @@ -2,6 +2,8 @@ from Acquisition import aq_base from Products.PluggableAuthService.UserPropertySheet import UserPropertySheet from Products.PluggableAuthService.interfaces.propertysheets import IPropertySheet # noqa +from node.utils import decode +from node.utils import encode from node.ext.ldap.interfaces import ILDAPGroupsConfig from node.ext.ldap.interfaces import ILDAPUsersConfig from zope.globalrequest import getRequest @@ -47,7 +49,7 @@ def __init__(self, principal, plugin): for key in self._attrmap: self._properties[key] = ldapprincipal.attrs.get(key, '') UserPropertySheet.__init__(self, principal.getId(), schema=None, - **self._properties) + **decode(self._properties)) def _get_ldap_principal(self): """returns ldap principal @@ -76,7 +78,8 @@ def setProperties(self, obj, mapping): assert(id in self._properties) ldapprincipal = self._get_ldap_principal() for id in mapping: - self._properties[id] = ldapprincipal.attrs[id] = mapping[id] + self._properties[id] = ldapprincipal.attrs[id] = encode( + mapping[id]) try: ldapprincipal.context() except Exception, e: From 3e0b5d37c19aac8cbdf04732e2083c3603233bcb Mon Sep 17 00:00:00 2001 From: Asko Soukka Date: Thu, 20 Oct 2016 14:39:19 +0300 Subject: [PATCH 08/17] Hardcode limits for searches and group listings --- src/pas/plugins/ldap/plugin.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/pas/plugins/ldap/plugin.py b/src/pas/plugins/ldap/plugin.py index 4aa1630..84e55af 100644 --- a/src/pas/plugins/ldap/plugin.py +++ b/src/pas/plugins/ldap/plugin.py @@ -9,6 +9,7 @@ from Products.PluggableAuthService.permissions import ManageGroups from Products.PluggableAuthService.permissions import ManageUsers from Products.PluggableAuthService.plugins.BasePlugin import BasePlugin +from Products.statusmessages.interfaces import IStatusMessage from node.utils import decode from node.utils import encode from node.ext.ldap.interfaces import ILDAPGroupsConfig @@ -295,6 +296,10 @@ def enumerateGroups(self, id=None, exact_match=False, sort_by=None, matches += batch_matches if not cookie: break + if len(matches) >= 100: + msg = 'Too many search results. Please, narrow your search.' + IStatusMessage(self.REQUEST).add(msg, type='warning') + return () if sort_by == 'id': matches = sorted(matches) pluginid = self.getId() @@ -440,6 +445,10 @@ def enumerateUsers(self, id=None, login=None, exact_match=False, matches += batch_matches if not cookie: break + if len(matches) >= 100: + msg = 'Too many search results. Please, narrow your search.' + IStatusMessage(self.REQUEST).add(msg, type='warning') + return () pluginid = self.getId() ret = list() for id, attrs in matches: @@ -709,7 +718,14 @@ def getGroupMembers(self, group_id): group = self.groups[group_id] except (KeyError, TypeError): return () - return tuple(group.member_ids) + member_ids = group.member_ids + if len(member_ids) <= 500: + return tuple(member_ids) + else: + msg = ('Group "{0:s}" has over 500 members. ' + 'Members cannot be listed.'.format(group_id)) + IStatusMessage(self.REQUEST).add(msg, type='error') + return () # ## # plonepas_interfaces.capabilities.IPasswordSetCapability From 09f20c0f06c635043c9a8162fdd9760e7ad4c045 Mon Sep 17 00:00:00 2001 From: Asko Soukka Date: Thu, 20 Oct 2016 14:42:44 +0300 Subject: [PATCH 09/17] Add hardcoded transform of 'Last, First' to 'First Last' in fullname --- src/pas/plugins/ldap/sheet.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/pas/plugins/ldap/sheet.py b/src/pas/plugins/ldap/sheet.py index 7a995f8..b1e3dbb 100644 --- a/src/pas/plugins/ldap/sheet.py +++ b/src/pas/plugins/ldap/sheet.py @@ -48,6 +48,9 @@ def __init__(self, principal, plugin): request['_ldap_props_reloaded'] = 1 for key in self._attrmap: self._properties[key] = ldapprincipal.attrs.get(key, '') + if 'fullname' in self._properties: + self._properties['fullname'] = ' '.join(map( + str.strip, reversed(self._properties['fullname'].split(',')))) UserPropertySheet.__init__(self, principal.getId(), schema=None, **decode(self._properties)) From 896675d6674740ba94e475858ad8673a129ea96f Mon Sep 17 00:00:00 2001 From: Asko Soukka Date: Fri, 21 Oct 2016 09:12:22 +0300 Subject: [PATCH 10/17] Add longer memoize for getGroupIds --- src/pas/plugins/ldap/plugin.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/pas/plugins/ldap/plugin.py b/src/pas/plugins/ldap/plugin.py index 84e55af..e6c1ef2 100644 --- a/src/pas/plugins/ldap/plugin.py +++ b/src/pas/plugins/ldap/plugin.py @@ -98,10 +98,10 @@ def _wrapper(self, *args, **kwargs): return _decorator -def cacheKey(func, *args, **kwargs): +def cacheKey(func, timeout, *args, **kwargs): key = __file__ key += ':' + func.__name__ - key += ':' + str(int(time.time() // 10)) + key += ':' + str(int(time.time() // timeout)) for arg in ([args] + list(kwargs.values())): if isinstance(arg, unicode): arg = arg.encode('utf-8') @@ -224,7 +224,7 @@ def authenticateCredentials(self, credentials): # Allow querying groups by ID, and searching for groups. # @security.private - @ram.cache(lambda f, o, *args, **kwargs: cacheKey(f, *args, **kwargs)) + @ram.cache(lambda f, o, *args, **kwargs: cacheKey(f, 10, *args, **kwargs)) def enumerateGroups(self, id=None, exact_match=False, sort_by=None, max_results=None, **kw): """ -> ( group_info_1, ... group_info_N ) @@ -323,7 +323,7 @@ def enumerateGroups(self, id=None, exact_match=False, sort_by=None, # # Determine the groups to which a user belongs. @security.private - @ram.cache(lambda f, o, *args, **kwargs: cacheKey(f, args[0])) + @ram.cache(lambda f, o, *args, **kwargs: cacheKey(f, 10, args[0])) def getGroupsForPrincipal(self, principal, request=None): """principal -> ( group_1, ... group_N ) @@ -357,7 +357,7 @@ def getGroupsForPrincipal(self, principal, request=None): # @ldap_error_handler('enumerateUsers') @security.private - @ram.cache(lambda f, o, *args, **kwargs: cacheKey(f, *args, **kwargs)) + @ram.cache(lambda f, o, *args, **kwargs: cacheKey(f, 10, *args, **kwargs)) def enumerateUsers(self, id=None, login=None, exact_match=False, sort_by=None, max_results=None, **kw): """-> ( user_info_1, ... user_info_N ) @@ -528,7 +528,7 @@ def removePrincipalFromGroup(self, principal_id, group_id): # which case the properties are not persistently mutable). # @security.private - @ram.cache(lambda f, o, *args, **kwargs: cacheKey(f, args[0])) + @ram.cache(lambda f, o, *args, **kwargs: cacheKey(f, 10, args[0])) def getPropertiesForUser(self, user_or_group, request=None): """User -> IMutablePropertySheet || {} @@ -686,7 +686,7 @@ def getGroups(self): """ return map(self.getGroupById, self.getGroupIds()) - @ram.cache(lambda f, o: cacheKey(f)) + @ram.cache(lambda f, o: cacheKey(f, 300)) def getGroupIds(self): """ Returns a list of the available groups (ids) From 698e642d85b8d83d03e0326f253b5b7647602be9 Mon Sep 17 00:00:00 2001 From: Asko Soukka Date: Fri, 21 Oct 2016 09:13:08 +0300 Subject: [PATCH 11/17] Optimize exact group matches to take a look at all group ids instead of doing a separate search --- src/pas/plugins/ldap/plugin.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/pas/plugins/ldap/plugin.py b/src/pas/plugins/ldap/plugin.py index e6c1ef2..676e3da 100644 --- a/src/pas/plugins/ldap/plugin.py +++ b/src/pas/plugins/ldap/plugin.py @@ -269,7 +269,15 @@ def enumerateGroups(self, id=None, exact_match=False, sort_by=None, groups = self.groups if not groups: return () - if id: + if id and exact_match: + if id in self.getGroupIds(): + return ({ + 'id': id, + 'pluginid': self.getId() + }) + else: + return () + elif id: kw['id'] = id if not kw: # show all matches = groups.ids @@ -280,11 +288,9 @@ def enumerateGroups(self, id=None, exact_match=False, sort_by=None, try: res = groups.search( criteria=encode(kw), - attrlist=exact_match and - groups.context.search_attrlist or None, + attrlist=None, exact_match=exact_match, - page_size=not exact_match and - self._ldap_props.page_size or None, + page_size=self._ldap_props.page_size, cookie=cookie, ) if isinstance(res, tuple): From befc47b65ae84620aed9783a4cc454ec7b13e824 Mon Sep 17 00:00:00 2001 From: Fred van Dijk Date: Tue, 13 Dec 2016 16:38:15 +0100 Subject: [PATCH 12/17] Skip matches that don't have our required login attribute. --- src/pas/plugins/ldap/plugin.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/pas/plugins/ldap/plugin.py b/src/pas/plugins/ldap/plugin.py index 676e3da..9107fdb 100644 --- a/src/pas/plugins/ldap/plugin.py +++ b/src/pas/plugins/ldap/plugin.py @@ -458,6 +458,8 @@ def enumerateUsers(self, id=None, login=None, exact_match=False, pluginid = self.getId() ret = list() for id, attrs in matches: + if 'login' not in attrs: + continue ret.append({ 'id': id, 'login': attrs['login'][0], From 789f07e899847df95856147a305d0c96b50f17e8 Mon Sep 17 00:00:00 2001 From: Maurits van Rees Date: Fri, 6 Jan 2017 18:52:45 +0100 Subject: [PATCH 13/17] Check if interface is active when calling a method. I had switched off all three group plugins, but got an error calling getGroupIds when accessing @@user-information. --- src/pas/plugins/ldap/plugin.py | 106 +++++++++++++++++++++++++++------ 1 file changed, 87 insertions(+), 19 deletions(-) diff --git a/src/pas/plugins/ldap/plugin.py b/src/pas/plugins/ldap/plugin.py index 9107fdb..f3b6f8d 100644 --- a/src/pas/plugins/ldap/plugin.py +++ b/src/pas/plugins/ldap/plugin.py @@ -136,6 +136,12 @@ def __init__(self, id, title=None, **kw): self.settings = OOBTree.OOBTree() self.plugin_caching = True + @security.private + def is_plugin_active(self, iface): + pas = self._getPAS() + ids = pas.plugins.listPluginIds(iface) + return self.getId() in ids + @property @security.private def groups_enabled(self): @@ -204,19 +210,22 @@ def authenticateCredentials(self, credentials): o If the credentials cannot be authenticated, return None. """ + default = None + if not self.is_plugin_active(pas_interfaces.IAuthenticationPlugin): + return default login = credentials.get('login') pw = credentials.get('password') if not (login and pw): - return None + return default logger.debug('credentials: %s' % credentials) users = self.users if not users: - return None + return default userid = users.authenticate(login, pw) if userid: logger.info('logged in %s' % userid) return (userid, login) - return None + return default # ## # pas_interfaces.IGroupEnumerationPlugin @@ -266,9 +275,12 @@ def enumerateGroups(self, id=None, exact_match=False, sort_by=None, o Insufficiently-specified criteria may have catastrophic scaling issues for some implementations. """ + default = () + if not self.is_plugin_active(pas_interfaces.IGroupEnumerationPlugin): + return default groups = self.groups if not groups: - return () + return default if id and exact_match: if id in self.getGroupIds(): return ({ @@ -276,7 +288,7 @@ def enumerateGroups(self, id=None, exact_match=False, sort_by=None, 'pluginid': self.getId() }) else: - return () + return default elif id: kw['id'] = id if not kw: # show all @@ -298,14 +310,14 @@ def enumerateGroups(self, id=None, exact_match=False, sort_by=None, else: batch_matches, cookie = res, '' except ValueError: - return tuple() + return default matches += batch_matches if not cookie: break if len(matches) >= 100: msg = 'Too many search results. Please, narrow your search.' IStatusMessage(self.REQUEST).add(msg, type='warning') - return () + return default if sort_by == 'id': matches = sorted(matches) pluginid = self.getId() @@ -338,9 +350,12 @@ def getGroupsForPrincipal(self, principal, request=None): o May assign groups based on values in the REQUEST object, if present """ + default = tuple() + if not self.is_plugin_active(pas_interfaces.IGroupsPlugin): + return default groups = self.groups if not groups: - return tuple() + return default _id = principal.getId() @@ -348,13 +363,13 @@ def getGroupsForPrincipal(self, principal, request=None): # XXX: that's where group in group will happen, but so far # group nodes do not provide membership info so we just # return if there is no user - return tuple() + return default users = self.users try: return users and users[_id].group_ids or [] except KeyError: pass - return tuple() + return default # ## # pas_interfaces.IUserEnumerationPlugin @@ -406,6 +421,9 @@ def enumerateUsers(self, id=None, login=None, exact_match=False, o Insufficiently-specified criteria may have catastrophic scaling issues for some implementations. """ + default = tuple() + if not self.is_plugin_active(pas_interfaces.IUserEnumerationPlugin): + return default # TODO: sort_by in node.ext.ldap if login: if not isinstance(login, basestring): @@ -424,7 +442,7 @@ def enumerateUsers(self, id=None, login=None, exact_match=False, kw['id'] = id users = self.users if not users: - return tuple() + return default if not exact_match: for value in users.principal_attrmap.values(): kw[value] = kw.values()[0] @@ -447,14 +465,14 @@ def enumerateUsers(self, id=None, login=None, exact_match=False, else: batch_matches, cookie = res, '' except ValueError: - return tuple() + return default matches += batch_matches if not cookie: break if len(matches) >= 100: msg = 'Too many search results. Please, narrow your search.' IStatusMessage(self.REQUEST).add(msg, type='warning') - return () + return default pluginid = self.getId() ret = list() for id, attrs in matches: @@ -468,6 +486,38 @@ def enumerateUsers(self, id=None, login=None, exact_match=False, ret = ret[:max_results] return ret + @security.private + def updateUser(self, user_id, login_name): + """ Update the login name of the user with id user_id. + + The plugin must return True (or any truth value) to indicate a + successful update, also when no update was needed. + + When updating a login name makes no sense for a plugin (most + likely because it does not actually store login names) and it + does not do anything, it must return None or False. + """ + # XXX + # if not self.is_plugin_active(pas_interfaces.IUserEnumerationPlugin): + # return default + return False + + @security.private + def updateEveryLoginName(self, quit_on_first_error=True): + """Update login names of all users to their canonical value. + + This should be done after changing the login_transform + property of PAS. + + You can set quit_on_first_error to False to report all errors + before quitting with an error. This can be useful if you want + to know how many problems there are, if any. + """ + # XXX + # if not self.is_plugin_active(pas_interfaces.IUserEnumerationPlugin): + # return default + return + # ## # plonepas_interfaces.group.IGroupManagement # @@ -548,13 +598,16 @@ def getPropertiesForUser(self, user_or_group, request=None): o May assign properties based on values in the REQUEST object, if present """ + default = {} + if not self.is_plugin_active(pas_interfaces.IPropertiesPlugin): + return default ugid = user_or_group.getId() try: if ugid in self.getGroupIds() or self.users[ugid]: return LDAPUserPropertySheet(user_or_group, self) except KeyError: pass - return {} + return default @security.private def setPropertiesForUser(self, user, propertysheet): @@ -659,9 +712,13 @@ def getGroupById(self, group_id): Returns the portal_groupdata-ish object for a group corresponding to this id. None if group does not exist here! """ + default = None + if not self.is_plugin_active( + plonepas_interfaces.group.IGroupIntrospection): + return default groups = self.groups if not groups or group_id not in self.getGroupIds(): - return None + return default ugmgroup = self.groups[group_id] title = ugmgroup.attrs.get('title', None) group = PloneGroup(ugmgroup.id, title).__of__(self) @@ -692,6 +749,9 @@ def getGroups(self): """ Returns an iteration of the available groups """ + # Checking self.is_plugin_active( + # plonepas_interfaces.group.IGroupIntrospection) + # is done in self.getGroupIds() already. return map(self.getGroupById, self.getGroupIds()) @ram.cache(lambda f, o: cacheKey(f, 300)) @@ -699,9 +759,13 @@ def getGroupIds(self): """ Returns a list of the available groups (ids) """ + default = [] + if not self.is_plugin_active( + plonepas_interfaces.group.IGroupIntrospection): + return default groups = self.groups if not groups: - return [] + return default matches = [] cookie = None while True: @@ -712,7 +776,7 @@ def getGroupIds(self): cookie=cookie, ) except ValueError: - return tuple() + return default matches += batch_matches if not cookie: break @@ -722,10 +786,14 @@ def getGroupMembers(self, group_id): """ return the members of the given group """ + default = () + if not self.is_plugin_active( + plonepas_interfaces.group.IGroupIntrospection): + return default try: group = self.groups[group_id] except (KeyError, TypeError): - return () + return default member_ids = group.member_ids if len(member_ids) <= 500: return tuple(member_ids) @@ -733,7 +801,7 @@ def getGroupMembers(self, group_id): msg = ('Group "{0:s}" has over 500 members. ' 'Members cannot be listed.'.format(group_id)) IStatusMessage(self.REQUEST).add(msg, type='error') - return () + return default # ## # plonepas_interfaces.capabilities.IPasswordSetCapability From 37462fd00e5b2d0c4973ca90c05d10d9e4b87d85 Mon Sep 17 00:00:00 2001 From: Maurits van Rees Date: Fri, 18 Aug 2017 11:47:34 +0200 Subject: [PATCH 14/17] Updated changelog --- CHANGES.rst | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGES.rst b/CHANGES.rst index 3ade612..5371b8e 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -5,7 +5,11 @@ History 1.5.2 (unreleased) ------------------ -- No changes yet. +- Check if interface is active when calling a method. [maurits] + +- Skip matches that don't have our required login attribute. [fredvd] + +- Merge Asko's speed optimizations. [asko] 1.5.1 (2016-10-18) From 7831b42290a1e392b6e01d4260b97bfbf65a23a9 Mon Sep 17 00:00:00 2001 From: Maurits van Rees Date: Fri, 18 Aug 2017 11:48:15 +0200 Subject: [PATCH 15/17] Avoid KeyError in getPropertiesForUser when self.users is None. --- CHANGES.rst | 2 ++ src/pas/plugins/ldap/plugin.py | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGES.rst b/CHANGES.rst index 5371b8e..c8a14b3 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -5,6 +5,8 @@ History 1.5.2 (unreleased) ------------------ +- Avoid ``KeyError`` in ``getPropertiesForUser`` when ``self.users`` is None. [maurits] + - Check if interface is active when calling a method. [maurits] - Skip matches that don't have our required login attribute. [fredvd] diff --git a/src/pas/plugins/ldap/plugin.py b/src/pas/plugins/ldap/plugin.py index f3b6f8d..e5dc646 100644 --- a/src/pas/plugins/ldap/plugin.py +++ b/src/pas/plugins/ldap/plugin.py @@ -602,8 +602,9 @@ def getPropertiesForUser(self, user_or_group, request=None): if not self.is_plugin_active(pas_interfaces.IPropertiesPlugin): return default ugid = user_or_group.getId() + users = self.users try: - if ugid in self.getGroupIds() or self.users[ugid]: + if ugid in self.getGroupIds() or users and users[ugid]: return LDAPUserPropertySheet(user_or_group, self) except KeyError: pass From 7718deafdcdc3e8331e58bfd34178e8334ed4b2c Mon Sep 17 00:00:00 2001 From: Fred van Dijk Date: Fri, 20 Oct 2017 12:19:28 +0200 Subject: [PATCH 16/17] Fix memcached setting str to unicode --- src/pas/plugins/ldap/properties.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/src/pas/plugins/ldap/properties.yaml b/src/pas/plugins/ldap/properties.yaml index 3915acd..7694d56 100644 --- a/src/pas/plugins/ldap/properties.yaml +++ b/src/pas/plugins/ldap/properties.yaml @@ -188,6 +188,7 @@ widgets: label: Memcached Server to use help: global - same server for all ldap plugins field.class: memcached field + datatype: integer - timeout: factory: '#field:number' value: expr:context.props.timeout From d0155d3789a6982a01101f870829bef3da34226c Mon Sep 17 00:00:00 2001 From: Fred van Dijk Date: Fri, 20 Oct 2017 12:54:41 +0200 Subject: [PATCH 17/17] It should be unicode, not integer --- src/pas/plugins/ldap/properties.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pas/plugins/ldap/properties.yaml b/src/pas/plugins/ldap/properties.yaml index 7694d56..1c6eb40 100644 --- a/src/pas/plugins/ldap/properties.yaml +++ b/src/pas/plugins/ldap/properties.yaml @@ -188,7 +188,7 @@ widgets: label: Memcached Server to use help: global - same server for all ldap plugins field.class: memcached field - datatype: integer + datatype: unicode - timeout: factory: '#field:number' value: expr:context.props.timeout