[Checkins] SVN: z3c.password/trunk/src/z3c/password/principal. merge -r 108748:110143 svn+ssh://svn.zope.org/repos/main/z3c.password/branches/jw-noraise-for-irrelevant-requests

Jan-Wijbrand Kolman janwijbrand at gmail.com
Wed Mar 24 05:18:05 EDT 2010


Log message for revision 110148:
  merge -r 108748:110143 svn+ssh://svn.zope.org/repos/main/z3c.password/branches/jw-noraise-for-irrelevant-requests

Changed:
  U   z3c.password/trunk/src/z3c/password/principal.py
  U   z3c.password/trunk/src/z3c/password/principal.txt

-=-
Modified: z3c.password/trunk/src/z3c/password/principal.py
===================================================================
--- z3c.password/trunk/src/z3c/password/principal.py	2010-03-24 09:14:33 UTC (rev 110147)
+++ z3c.password/trunk/src/z3c/password/principal.py	2010-03-24 09:18:04 UTC (rev 110148)
@@ -81,6 +81,30 @@
         #hook to facilitate testing and easier override
         return datetime.datetime.now()
 
+    def _isRelevantRequest(self):
+        fac = self._failedAttemptCheck()
+        if fac is None:
+            return True
+
+        if fac == interfaces.TML_CHECK_ALL:
+            return True
+
+        interaction = getInteraction()
+        try:
+            request = interaction.participations[0]
+        except IndexError:
+            return True # no request, we regard that as relevant.
+
+        if fac == interfaces.TML_CHECK_NONRESOURCE:
+            if '/@@/' in request.getURL():
+                return False
+            return True
+
+        if fac == interfaces.TML_CHECK_POSTONLY:
+            if request.method == 'POST':
+                return True
+            return False
+
     def checkPassword(self, pwd, ignoreExpiration=False, ignoreFailures=False):
         # keep this as fast as possible, because it will be called (usually)
         # for EACH request
@@ -88,6 +112,11 @@
         # Check the password
         same = super(PrincipalMixIn, self).checkPassword(pwd)
 
+        # Do not try to record failed attempts or raise account locked
+        # errors for requests that are irrelevant in this regard.
+        if not self._isRelevantRequest():
+            return same
+
         if not ignoreFailures and self.lastFailedAttempt is not None:
             if self.tooManyLoginFailures():
                 locked = self.accountLocked()
@@ -118,7 +147,9 @@
             add = 0
         else:
             #failed attempt, record it, increase counter
-            add = self.checkFailedAttempt()
+            self.failedAttempts += 1
+            self.lastFailedAttempt = self.now()
+            add = 1
 
         # If the maximum amount of failures has been reached notify the
         # system by raising an error.
@@ -133,45 +164,6 @@
 
         return same
 
-    def _getRequest(self):
-        interaction = getInteraction()
-        try:
-            return interaction.participations[0]
-        except IndexError:
-            return None
-
-    def checkFailedAttempt(self):
-        #failed attempt, record it, increase counter (in case we have to)
-        validRequest = True
-        fac = self._failedAttemptCheck()
-        if fac == interfaces.TML_CHECK_ALL:
-            validRequest = True
-        else:
-            request = self._getRequest()
-            if request is None:
-                validRequest = True
-            else:
-                if fac == interfaces.TML_CHECK_NONRESOURCE:
-                    url = request.getURL()
-                    if '/@@/' in url:
-                        #this is a resource
-                        validRequest = False
-                    else:
-                        validRequest = True
-                elif fac == interfaces.TML_CHECK_POSTONLY:
-                    if request.method == 'POST':
-                        #this is a POST request
-                        validRequest = True
-                    else:
-                        validRequest = False
-
-        if validRequest:
-            self.failedAttempts += 1
-            self.lastFailedAttempt = self.now()
-            return 1
-        else:
-            return 0
-
     def tooManyLoginFailures(self, add = 0):
         attempts = self._maxFailedAttempts()
         #this one needs to be >=, because... data just does not

Modified: z3c.password/trunk/src/z3c/password/principal.txt
===================================================================
--- z3c.password/trunk/src/z3c/password/principal.txt	2010-03-24 09:14:33 UTC (rev 110147)
+++ z3c.password/trunk/src/z3c/password/principal.txt	2010-03-24 09:18:04 UTC (rev 110148)
@@ -196,7 +196,7 @@
   >>> user.failedAttemptCheck = interfaces.TML_CHECK_NONRESOURCE
 
 Create our dummy request:
-Watch out! this is a request for a resource (/@@/)
+Watch out! this is a request for a resource (note the "/@@/" in the URL)
 
   >>> request = testing.TestBrowserRequest('http://localhost/@@/logo.gif')
   >>> zope.security.management.getInteraction().add(request)
@@ -273,7 +273,6 @@
   0
 
 Try a POST request. What a loginform usually is.
-(Note, that the request gets examined only if the password does not match.)
 
   >>> zope.security.management.getInteraction().remove(request)
   >>> request = testing.TestBrowserRequest('http://localhost/loginform.html',
@@ -614,7 +613,6 @@
   0
 
 Try a POST request. What a loginform usually is.
-(Note, that the request gets examined only if the password does not match.)
 
   >>> zope.security.management.getInteraction().remove(request)
   >>> request = testing.TestBrowserRequest('http://localhost/loginform.html',
@@ -1007,8 +1005,44 @@
   >>> user.lastFailedAttempt is None
   True
 
+After the maximum amount of failed attempts has been reached, subsequent
+login attempts will raise an error. This error should however only be raised
+for these types of requests that were relevant to the counting failed
+attempts.
 
+  >>> # Set a POST request, as only this type of request will be counted for.
+  >>> request = testing.TestBrowserRequest(
+  ...    'http://localhost/index.html', 'POST')
+  >>> zope.security.management.getInteraction().add(request)
 
+  >>> poptions.failedAttemptCheck = interfaces.TML_CHECK_POSTONLY
+  >>> poptions.maxFailedAttempts = 2
+  >>> user = MyPrincipal('srichter', '123123', u'Stephan Richter')
+  >>> user.checkPassword('wrong_once')
+  False
+  >>> user.failedAttempts
+  1
+
+  >>> user.checkPassword('wrong_twice')
+  False
+  >>> user.failedAttempts
+  2
+
+  >>> user.checkPassword('wrong_three_times')
+  Traceback (most recent call last):
+  ...
+  TooManyLoginFailures: The password was entered incorrectly too often.
+
+  >>> # Set a GET request. This should not raise any error.
+  >>> zope.security.management.getInteraction().remove(request)
+  >>> request = testing.TestBrowserRequest(
+  ...    'http://localhost/@@/logo.gif', 'GET')
+  >>> zope.security.management.getInteraction().add(request)
+
+  >>> user.checkPassword('wrong_four_times')
+  False
+
+
 ``passwordSetOn`` might happen to be None.
 In case the mixin gets applied to the user object after it's been created
 the ``passwordSetOn`` property will be None. That caused a bug.



More information about the checkins mailing list