Hello Jan-Wijbrand,

Thinking this further, resource requests will have a '/@@/' (usually) in their URL. So it might be an idea to skip those requests from counting.

Tuesday, January 19, 2010, 10:43:43 AM, you wrote:

JWK> Adam GROSZER agroszer@gmail.com wrote:

JWK> Thank you for your reply!

...

Right. Well right now the lame solution is to try to count the number of resources needed to load the page and multiply maxFailedAttempts by that number. Other than that you _will_ want to offload resources in production.

JWK> Oh, sure in _production_ I will. But what about testing and development. And JWK> counting the number of resources is highly unpredictable - esp. as I'm working JWK> on applications that can be easily skinned for different clients.

...

(An other idea could be to try to identify requests that are not for resources, but how?)

JWK> There's no real way as far as I can tell :|

JWK> Even worse, if Zope were to handle one or more login page requisites, you can JWK> easily get database write conflicts as multiple "simultaneous" requests will try JWK> to increase the failed login attempts counter on the internal principal object.

JWK> Can I conclude this particular feature of z3c.password is not really in use by JWK> anyone else (at least not in combination with the SessionCredentialsPlugin)?

JWK> regards, JWK> jw

...

Tuesday, January 19, 2010, 12:16:38 AM, you wrote:

...

...

Betreff: [Zope-dev] z3c.password and "failedAttempts" andSessionCredentialsPlugin

Hi,

I'm looking into integrating z3c.password into an application that also uses the PAU with the principal folder, InternalPrincipals and the SessionCredentialsPlugin.

One of the features of z3c.password that I'd like to use is locking out an user account after a number of failed login attempts. z3c.password provides such a feature.

However, it seems this feature does not play well with the SessionCredentialsPlugin:

The SessionCredentials will store the login and password that were submitted through the login form in a session. This login/password combination then is checked against the internal principal stored in the principal folder.

If the password is correct then (obviously) everything is fine.

When the password is incorrect, the user is directed back to the login form as authentication failed. If the page with the login form retrieves resources (like images or css of javascript files) and since the zopepublication will _try_ to authenticate every request, the wrongful login/password combination is checked multiple times for that page and its resources against the internal principal object.

The internal principal mixin of z3c.password will count the number of failed checks. If you tell it to lock out an user after, say, three failed attempts, you have a problem, as the number of login page resources (thus requests) will quite easily outnumber the maximum number of attempts.

Questions: is anyone using this feature of z3c.password in combination with the SessionCredentialsPlugin? If this is working for you, do you have any idea what I am doing wrong here? What type of authentication are the authors of z3c.password using?

R> Probably Adam can tell you more about that.

R> One solution could be to offload your resources and R> deliver them from Apache or Nginx Frontend.

R> Regards R> Roger Ineichen

...

...

Thanks for any insight here. regards, jw

---

Zope-Dev maillist - Zope-Dev@zope.org https://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - https://mail.zope.org/mailman/listinfo/zope-announce https://mail.zope.org/mailman/listinfo/zope )

R> _______________________________________________ R> Zope-Dev maillist - Zope-Dev@zope.org R> https://mail.zope.org/mailman/listinfo/zope-dev R> ** No cross posts or HTML encoding! ** R> (Related lists - R> https://mail.zope.org/mailman/listinfo/zope-announce R> https://mail.zope.org/mailman/listinfo/zope )

-- Best regards, Adam GROSZER mailto:agroszer@gmail.com

JWK> _______________________________________________ JWK> Zope-Dev maillist - Zope-Dev@zope.org JWK> https://mail.zope.org/mailman/listinfo/zope-dev JWK> ** No cross posts or HTML encoding! ** JWK> (Related lists - JWK> https://mail.zope.org/mailman/listinfo/zope-announce JWK> https://mail.zope.org/mailman/listinfo/zope )

Hi Adam,

Tres Seaver tseaver@palladion.com wrote:

-----BEGIN PGP SIGNED MESSAGE----- Adam GROSZER wrote:

...

Thinking this further, resource requests will have a '/@@/' (usually) in their URL. So it might be an idea to skip those requests from counting.

Seems to me skipping anything except POST requests would be the right thing to do: suppose the user reloads the login form for some reason (maybe their browser crashes, whatever).

I see you're working on this issue. That's fantastic. If you need a tester... by all means let me know :-)

regards, jw

Hi Adam,

Adam GROSZER agroszer@gmail.com wrote:

We're using it too... Go for 0.8.0.

I noticed that requesting for, for example, a resource could still trigger account locked errors, where in my opinion only the login attempts themselves should do that.

I created a branch...

http://svn.zope.org/z3c.password/branches/jw-noraise-for-irrelevant-requests...

..that will check for request relevancy as early as possible.

All tests pass without modification, but still, after an account has been locked requests for, for example resources, will still work (the diff against the trunk is pasted below). Additionally I think the code is tad more readable now.

I realize now that I should add a test to demonstrate this.

Anyway, could you tell me what you think of this approach?

regards, jw

--- z3c.password-trunk/src/z3c/password/principal.py 2010-02-04 12:15:46.632785192 +0100 +++ z3c.password/src/z3c/password/principal.py 2010-02-04 12:08:56.351805021 +0100 @@ -13,7 +13,7 @@ ############################################################################## """Principal MixIn for Advanced Password Management

-$Id: principal.py 108644 2010-01-29 17:28:55Z adamg $ +$Id: principal.py 108749 2010-02-04 11:08:55Z janwijbrand $ """ __docformat__ = "reStructuredText" import datetime @@ -81,6 +81,30 @@ #hook to facilitate testing and easier override return datetime.datetime.now()

+ def _isIrrelevantRequest(self, RELEVANT=False, IRRELEVANT=True): + fac = self._failedAttemptCheck() + if fac is None: + return RELEVANT + + if fac == interfaces.TML_CHECK_ALL: + return RELEVANT + + interaction = getInteraction() + try: + request = interaction.participations[0] + except IndexError: + return RELEVANT # no request, we regard that as relevant. + + if fac == interfaces.TML_CHECK_NONRESOURCE: + if '/@@/' in request.getURL(): + return IRRELEVANT + return RELEVANT + + if fac == interfaces.TML_CHECK_POSTONLY: + if request.method == 'POST': + return RELEVANT + return IRRELEVANT + 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 self._isIrrelevantRequest(): + 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 -