[Checkins] SVN: Products.PluggableAuthService/trunk/Products/PluggableAuthService/ Launchpad #273680: Avoid expensive / incorrect dive into 'enumerateUsers'

Tres Seaver tseaver at palladion.com
Thu Nov 20 10:18:01 EST 2008


Log message for revision 93164:
  Launchpad #273680:  Avoid expensive / incorrect dive into 'enumerateUsers'
  when trying to validate w/o either a real ID or login.
  

Changed:
  U   Products.PluggableAuthService/trunk/Products/PluggableAuthService/PluggableAuthService.py
  U   Products.PluggableAuthService/trunk/Products/PluggableAuthService/doc/CHANGES.txt
  U   Products.PluggableAuthService/trunk/Products/PluggableAuthService/tests/test_PluggableAuthService.py

-=-
Modified: Products.PluggableAuthService/trunk/Products/PluggableAuthService/PluggableAuthService.py
===================================================================
--- Products.PluggableAuthService/trunk/Products/PluggableAuthService/PluggableAuthService.py	2008-11-20 15:16:45 UTC (rev 93163)
+++ Products.PluggableAuthService/trunk/Products/PluggableAuthService/PluggableAuthService.py	2008-11-20 15:18:00 UTC (rev 93164)
@@ -758,9 +758,13 @@
 
     security.declarePrivate( '_verifyUser' )
     def _verifyUser( self, plugins, user_id=None, login=None ):
-
         """ user_id -> info_dict or None
         """
+        if user_id is None and login is None:
+            # Avoid possible hugely expensive and/or wrong behavior of
+            # plugin enumerators.
+            return None
+
         criteria = {'exact_match': True}
 
         if user_id is not None:
@@ -769,35 +773,34 @@
         if login is not None:
             criteria[ 'login' ] = login
 
-        if criteria:
-            view_name = createViewName('_verifyUser', user_id or login)
-            keywords = createKeywords(**criteria)
-            cached_info = self.ZCacheable_get( view_name=view_name
-                                             , keywords=keywords
-                                             , default=None
-                                             )
+        view_name = createViewName('_verifyUser', user_id or login)
+        keywords = createKeywords(**criteria)
+        cached_info = self.ZCacheable_get( view_name=view_name
+                                            , keywords=keywords
+                                            , default=None
+                                            )
 
-            if cached_info is not None:
-                return cached_info
+        if cached_info is not None:
+            return cached_info
 
 
-            enumerators = plugins.listPlugins( IUserEnumerationPlugin )
+        enumerators = plugins.listPlugins( IUserEnumerationPlugin )
 
-            for enumerator_id, enumerator in enumerators:
-                try:
-                    info = enumerator.enumerateUsers( **criteria )
+        for enumerator_id, enumerator in enumerators:
+            try:
+                info = enumerator.enumerateUsers( **criteria )
 
-                    if info:
-                        # Put the computed value into the cache
-                        self.ZCacheable_set( info[0]
-                                           , view_name=view_name
-                                           , keywords=keywords
-                                           )
-                        return info[0]
+                if info:
+                    # Put the computed value into the cache
+                    self.ZCacheable_set( info[0]
+                                        , view_name=view_name
+                                        , keywords=keywords
+                                        )
+                    return info[0]
 
-                except _SWALLOWABLE_PLUGIN_EXCEPTIONS:
-                    msg = 'UserEnumerationPlugin %s error' % enumerator_id
-                    logger.debug(msg, exc_info=True)
+            except _SWALLOWABLE_PLUGIN_EXCEPTIONS:
+                msg = 'UserEnumerationPlugin %s error' % enumerator_id
+                logger.debug(msg, exc_info=True)
 
         return None
 

Modified: Products.PluggableAuthService/trunk/Products/PluggableAuthService/doc/CHANGES.txt
===================================================================
--- Products.PluggableAuthService/trunk/Products/PluggableAuthService/doc/CHANGES.txt	2008-11-20 15:16:45 UTC (rev 93163)
+++ Products.PluggableAuthService/trunk/Products/PluggableAuthService/doc/CHANGES.txt	2008-11-20 15:18:00 UTC (rev 93164)
@@ -4,6 +4,9 @@
 PluggableAuthService 1.7 (unreleased)
 -------------------------------------
 
+- Launchpad #273680:  Avoid expensive / incorrect dive into 'enumerateUsers'
+  when trying to validate w/o either a real ID or login.
+
 - Launchpad #300321:  ZODBGroupManager.enumerateGroups failed to find
   groups with unicode IDs.
 

Modified: Products.PluggableAuthService/trunk/Products/PluggableAuthService/tests/test_PluggableAuthService.py
===================================================================
--- Products.PluggableAuthService/trunk/Products/PluggableAuthService/tests/test_PluggableAuthService.py	2008-11-20 15:16:45 UTC (rev 93163)
+++ Products.PluggableAuthService/trunk/Products/PluggableAuthService/tests/test_PluggableAuthService.py	2008-11-20 15:18:00 UTC (rev 93164)
@@ -61,6 +61,7 @@
 
         return ()
 
+
 class DummyMultiUserEnumerator( DummyPlugin ):
 
     def __init__( self, pluginid, *users ):
@@ -100,6 +101,11 @@
 
         return tuple(results)
 
+class WantonUserEnumerator(DummyMultiUserEnumerator):
+    def enumerateUsers( self, *args, **kw):
+        # Always returns everybody.
+        return self.users
+
 class DummyGroupEnumerator( DummyPlugin ):
 
     def __init__( self, group_id ):
@@ -1105,6 +1111,31 @@
         self.failUnless(
             zcuf._verifyUser(plugins, login='foobar')['id'] == 'foo')
 
+    def test__verifyUser_no_login_or_userid( self ):
+        # setup cargo-culted from other tests...
+        from Products.PluggableAuthService.interfaces.plugins \
+             import IUserEnumerationPlugin
+        plugins = self._makePlugins()
+        zcuf = self._makeOne( plugins )
+
+        users = ({'id': 'foo', 'login': 'foobar'},
+                 {'id': 'bar', 'login': 'foo'})
+        enumerator = WantonUserEnumerator('enumerator', *users)
+        directlyProvides( enumerator, IUserEnumerationPlugin )
+        zcuf._setObject( 'enumerator', enumerator )
+        plugins = zcuf._getOb( 'plugins' )
+        plugins.activatePlugin( IUserEnumerationPlugin, 'enumerator' )
+
+        # Our enumerator plugin normally returns something, even if
+        # you ask for a nonexistent user.
+        self.failUnless(zcuf._verifyUser(plugins, login='qux') in users)
+        
+        # But with no criteria, we should always get None.
+        self.assertEqual(zcuf._verifyUser(plugins, login=None, user_id=None),
+                         None)
+        self.assertEqual(zcuf._verifyUser(plugins), None)
+
+
     def test__findUser_no_plugins( self ):
 
         plugins = self._makePlugins()



More information about the Checkins mailing list