merge -r 108748:110143 svn+ssh://svn.zope.org/repos/main/z3c.password…

…/branches/jw-noraise-for-irrelevant-requests

src/z3c/password/principal.py

@@ -81,13 +81,42 @@ def now(self):
         #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
 
         # 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 @@ def checkPassword(self, pwd, ignoreExpiration=False, ignoreFailures=False):
             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 @@ def checkPassword(self, pwd, ignoreExpiration=False, ignoreFailures=False):
 
         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

src/z3c/password/principal.txt

@@ -196,7 +196,7 @@ Set the option on the user:
   >>> 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 @@ But the counter is not incremented.
   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 @@ But the counter is not incremented.
   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,6 +1005,42 @@ Gets reset on a successful check:
   >>> 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.