I think this is duplicate (or better triplicate) of: [ticket:3043] [ticket:3064]

I'll try to summarize them here.

== Issue ==
Password quality is not checked

== Possible solutions ==
Use of cracklib or [https://fedorahosted.org/libpwquality libpwquality]

== Where to patch ==
This should probably be integrated at the !PasswordStrength class of [http://git.fedorahosted.org/git/?p=fas.git;a=blob;f=fas/validators.py;hb=HEAD#l231 fas/validator]

I've closed the other tickets, lets use this one to track this work.

The following patch should do the trick.

I don't know how to test it so i'll wait for an ACK before commit/push..

{{{
diff --git a/fas/validators.py b/fas/validators.py
index 6036eba..be87958 100644
--- a/fas/validators.py
+++ b/fas/validators.py
@@ -41,6 +41,8 @@ from fas.util import available_languages

from fas.model import People, Groups

+import pwquality
+
### HACK: TurboGears/FormEncode requires that we use a dummy _ function for
# error messages.
# http://docs.turbogears.org/1.0/Internationalization#id13
@@ -239,6 +241,13 @@ class PasswordStrength(validators.UnicodeString):
if value.lower() in (u'correct horse battery staple',
u'correcthorsebatterystaple', u'tr0ub4dor&3'):
raise validators.Invalid(self.message('xkcd', state), value, state)
+
+ try:
+ pw_quality = pwquality.PWQSettings()
+ pw_quality.read_config()
+ pw_quality.check(value, None, None)
+ except pwquality.PWQError as (e, msg):
+ raise validators.Invalid(msg, value, state)

     diversity = set(value)
     if len(diversity) < 2:

}}}

Small update:
- in format-patch (as pingou asked)
- with testing whether the lib is installed (otherwise the check is silently ignored) (nirik's comment)

As before, i'll wait for an ACK before push.

{{{
From bdeddbaf8880d58090f0ebf375e12bc1f32fa8a5 Mon Sep 17 00:00:00 2001
From: Christos Triantafyllidis christos.triantafyllidis@gmail.com
Date: Fri, 9 Mar 2012 21:09:19 +0200
Subject: [PATCH 1/2] Check the FAS password against dictionary words using
libpwquality

fas/validators.py | 14 ++++++++++++++
1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/fas/validators.py b/fas/validators.py
index 6036eba..a56ea0a 100644
--- a/fas/validators.py
+++ b/fas/validators.py
@@ -41,6 +41,12 @@ from fas.util import available_languages

from fas.model import People, Groups

+from sys import modules
+try:
+ import pwquality
+except ImportError:
+ pass
+
### HACK: TurboGears/FormEncode requires that we use a dummy _ function for
# error messages.
# http://docs.turbogears.org/1.0/Internationalization#id13
@@ -239,6 +245,14 @@ class PasswordStrength(validators.UnicodeString):
if value.lower() in (u'correct horse battery staple',
u'correcthorsebatterystaple', u'tr0ub4dor&3'):
raise validators.Invalid(self.message('xkcd', state), value, state)
+
+ if "pwquality" in modules.keys():
+ try:
+ pw_quality = pwquality.PWQSettings()
+ pw_quality.read_config()
+ pw_quality.check(value, None, None)
+ except pwquality.PWQError as (e, msg):
+ raise validators.Invalid(msg, value, state)

     diversity = set(value)
     if len(diversity) < 2:

--
1.7.4.4

From b080c0e804fd34b3f6abeac4e38b17b94dfa617b Mon Sep 17 00:00:00 2001
From: Christos Triantafyllidis christos.triantafyllidis@gmail.com
Date: Fri, 9 Mar 2012 21:50:23 +0200
Subject: [PATCH 2/2] Added python-pwquality in INSTALL dependancies

INSTALL | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/INSTALL b/INSTALL
index 555336a..8a2676b 100644
--- a/INSTALL
+++ b/INSTALL
@@ -23,7 +23,8 @@ As a privileged user on a Fedora system run the following:
yum install git-core postgresql-plpython postgresql-server postgresql-python \
python-TurboMail TurboGears pygpgme python-sqlalchemy python-genshi \
python-psycopg2 pytz python-babel babel python-GeoIP python-openid \
- python-fedora-turbogears python-migrate python-memcached python-tgcaptcha2 pyOpenSSL gettext
+ python-fedora-turbogears python-migrate python-memcached python-tgcaptcha2 \
+ python-pwquality pyOpenSSL gettext

NOTE: Please see the HACKING file located in the same directory as this file for caveats specific to various Fedora and RedHat Enterprise Linux releases.

--
1.7.4.4

}}}

Pretty good -- two changes I'd make:

Change this:

if "pwquality" in modules.keys():

To this:

if "pwquality" in modules:

(keys() creates a list. We don't need that temporary list here.)

It's nicer if we can pass the message through the validator's message dict so that it can be translated. OTOH, pwquality's message is probably more informative. Does something like this work:

{{{

messages = {[...],
'pwquality': _(r'libpwquality reports this is a weak password: %(pwq_msg)s'),
}

[...]
except pwquality.PWQError as (e, msg):
raise validators.Invalid(self.message('pwquality', state) % {'pwq_msg': msg}, value, state)

}}}

If it doesn't work, just add a comment to your code that it would be nice to find a way to get the exception message from the validator itself (so it's translated for our app).

ctria: Are you able to implement toshio's suggestions and push this to the fas repo?

I'm trying to get a release ready, and it'd be nice to get this in beforehand, so translators can translate the strings and so on.

Any update here? :)

attachment
0001-Check-the-FAS-password-against-dictionary-words-usin.patch

Hi,

i did as toshio suggests. I'm not able to test this in FAS environment (did some command line testing thought and works fine).

I'm not pushing, i've attached the patch to the ticket.

Cheers and sorry for the delay,

Christos

I just read the patch (no test performed) but I feel there will be a problem if pwquality isn't installed.

At the top you pass if the import fails but you don't do anything with this. So if the import fails, I expect the actual checking part to fail as well and not with a pwquality.PWQError exception.

I would create a boolean with_pwquality = True at the top, try to import, turn the boolean to False if the import fails and run the pyquality bits only if the boolean is True. That or just don't catch the exception on import and let FAS fail plainly for missing dependency.

Toshio/codeblock a preference ?

Forget what I said it's already been taken care of line 250 :-)

Looks good. We'll want to add an entry to the NEWS file about using pwquality if that is installed.

This patch does add a string, though. So if CodeBlock has already sent a message to translators we need to hold off on pushing this until after string freeze/the next release.

codeBlock -- do you want to have this committed in a branch? Or just remember to push it after the release?

I haven't emailed trans@ yet, have been waiting on this patch.

I will push this into master and email trans today. This was the last blocker before I sent off the email.

This has been pushed to the FAS master branch, hash 5e0f32d.

I'm going to close this out - this will be in the next fas release.