[Checkins] SVN: Products.PluggableAuthService/branches/slinkp-fix-anonymous-performance-1.5-branch/Products/PluggableAuthService/ Fix https://bugs.launchpad.net/zope-pas/+bug/273680 for the 1.5 branch, now

[Paul Winkler]

Log message for revision 91441:
  Fix https://bugs.launchpad.net/zope-pas/+bug/273680 for the 1.5 branch, now
  with a proper commit message and CHANGES.txt entry: _verifyUser() for
  Anonymous could result in very expensive lookup of all users for some
  implementations, eg. Membrane.
  

Changed:
  U   Products.PluggableAuthService/branches/slinkp-fix-anonymous-performance-1.5-branch/Products/PluggableAuthService/PluggableAuthService.py
  U   Products.PluggableAuthService/branches/slinkp-fix-anonymous-performance-1.5-branch/Products/PluggableAuthService/doc/CHANGES.txt
  U   Products.PluggableAuthService/branches/slinkp-fix-anonymous-performance-1.5-branch/Products/PluggableAuthService/tests/test_PluggableAuthService.py

-=-
Modified: Products.PluggableAuthService/branches/slinkp-fix-anonymous-performance-1.5-branch/Products/PluggableAuthService/PluggableAuthService.py
===================================================================
--- Products.PluggableAuthService/branches/slinkp-fix-anonymous-performance-1.5-branch/Products/PluggableAuthService/PluggableAuthService.py	2008-09-24 15:58:19 UTC (rev 91440)
+++ Products.PluggableAuthService/branches/slinkp-fix-anonymous-performance-1.5-branch/Products/PluggableAuthService/PluggableAuthService.py	2008-09-24 15:58:36 UTC (rev 91441)
@@ -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,7 +773,7 @@
         if login is not None:
             criteria[ 'login' ] = login
 
-        if criteria:
+        if criteria:  # Um, this is always true.
             view_name = createViewName('_verifyUser', user_id or login)
             keywords = createKeywords(**criteria)
             cached_info = self.ZCacheable_get( view_name=view_name

Modified: Products.PluggableAuthService/branches/slinkp-fix-anonymous-performance-1.5-branch/Products/PluggableAuthService/doc/CHANGES.txt
===================================================================
--- Products.PluggableAuthService/branches/slinkp-fix-anonymous-performance-1.5-branch/Products/PluggableAuthService/doc/CHANGES.txt	2008-09-24 15:58:19 UTC (rev 91440)
+++ Products.PluggableAuthService/branches/slinkp-fix-anonymous-performance-1.5-branch/Products/PluggableAuthService/doc/CHANGES.txt	2008-09-24 15:58:36 UTC (rev 91441)
@@ -4,6 +4,10 @@
 PluggableAuthService 1.5.4 (Unreleased)
 ---------------------------------------
 
+- Launchpad #273680: _verifyUser() for Anonymous could result in very
+  expensive lookup of all users for some implementations,
+  eg. Membrane.
+
 - Ensure the _findUser cache is invalidated if the roles or groups for
   a principal change.
 

Modified: Products.PluggableAuthService/branches/slinkp-fix-anonymous-performance-1.5-branch/Products/PluggableAuthService/tests/test_PluggableAuthService.py
===================================================================
--- Products.PluggableAuthService/branches/slinkp-fix-anonymous-performance-1.5-branch/Products/PluggableAuthService/tests/test_PluggableAuthService.py	2008-09-24 15:58:19 UTC (rev 91440)
+++ Products.PluggableAuthService/branches/slinkp-fix-anonymous-performance-1.5-branch/Products/PluggableAuthService/tests/test_PluggableAuthService.py	2008-09-24 15:58:36 UTC (rev 91441)
@@ -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()