Merge lp:~camptocamp/openobject-addons/improve_auth_crypt_3_please_launchpad_work-nbi into lp:openobject-addons/7.0

[Nicolas Bessi - Camptocamp]

[IMP] Add an init function on res.users to encrypt all passwords when installing module and avoid plain passwords for deactivated users.

([IMP] module auth_crypt use sha256 by default to encrypt password. The modification keeps retro compatibility.) REMOVED as disscussed with Olivier

[IMP] Add an init function on res.users to encrypt all passwords when installing module and avoid plain password for deactivated users.

[Olivier Dony (Odoo) (odo-openerp)]

Hi,

As discussed in bug 1280152 comments, we should consider these as 2 separate issues, so this MP could start by dealing with the first one (the lazy password encryption issue).

For this part, your init() method looks fine, but there are already multiple instances of the salting+hashing dance. As you're adding one more, it seems a good opportunity to refactor a bit and extract that pattern into a private method, something like:

def _set_user_password(self, cr, uid, user_id, password, context=None):
     password_hash = md5_crypt(password, gen_salt()) # TODO: update default algo in trunk
     cr.execute("UPDATE res_users SET password='', password_crypt=%s WHERE id=%s",
                (password_hash, user_id))

Thanks!

review: Needs Fixing

[Nicolas Bessi - Camptocamp (nbessi-c2c-deactivatedaccount)]

Hello,

I have made the modification as required only the init function is in the MP

[Nicolas Bessi - Camptocamp (nbessi-c2c-deactivatedaccount)]

Hello Olivier,

Any news on this one

Regards

Nicolas

[Olivier Dony (Odoo) (odo-openerp)]

Thanks for the update! Moved to github for merge (added guard for NULL passwords): https://github.com/odoo/odoo/pull/628

[Olivier Dony (Odoo) (odo-openerp)]

Merged in 7.0 at https://github.com/odoo-dev/odoo/commit/f29ff5ef, so closing it.

=== modified file 'auth_crypt/auth_crypt.py'
--- auth_crypt/auth_crypt.py	2013-08-01 16:27:04 +0000
+++ auth_crypt/auth_crypt.py	2014-03-19 14:52:30 +0000
@@ -105,6 +105,7 @@
 
     return magic + salt + '$' + rearranged
 
+
 def sh256crypt(cls, password, salt, magic=magic_sha256):
     iterations = 1000
     # see http://en.wikipedia.org/wiki/PBKDF2
@@ -114,16 +115,41 @@
     result = result.encode('base64') # doesnt seem to be crypt(3) compatible
     return '%s%s$%s' % (magic_sha256, salt, result)
 
+
 class res_users(osv.osv):
     _inherit = "res.users"
 
+    def init(self, cr):
+        """Encrypt all passwords at module installation"""
+        cr.execute("SELECT id, password FROM res_users WHERE password != ''",)
+        to_encrypt = cr.fetchall()
+        if to_encrypt:
+            for user in to_encrypt:
+                self._set_encrypted_password(cr, user[0], user[1])
+        return True
+
+    def _set_encrypted_password(self, cr, user_id, plain_password):
+        """Set an encrypted password for a given user
+
+        :param cr: database cursor
+        :param user_id: user_id
+        :param plain_password: plain password to encrypt
+
+        :returns: user id
+
+        """
+        salt = gen_salt()
+        stored_password_crypt = md5crypt(plain_password, salt)
+        cr.execute("UPDATE res_users SET password='', password_crypt=%s WHERE id=%s",
+                   (stored_password_crypt, user_id))
+        return user_id
+
     def set_pw(self, cr, uid, id, name, value, args, context):
         if value:
-            encrypted = md5crypt(value, gen_salt())
-            cr.execute("update res_users set password='', password_crypt=%s where id=%s", (encrypted, id))
+            self._set_encrypted_password(cr, id, value)
         del value
 
-    def get_pw( self, cr, uid, ids, name, args, context ):
+    def get_pw(self, cr, uid, ids, name, args, context):
         cr.execute('select id, password from res_users where id in %s', (tuple(map(int, ids)),))
         stored_pws = cr.fetchall()
         res = {}
@@ -144,9 +170,7 @@
         if cr.rowcount:
             stored_password, stored_password_crypt = cr.fetchone()
             if stored_password and not stored_password_crypt:
-                salt = gen_salt()
-                stored_password_crypt = md5crypt(stored_password, salt)
-                cr.execute("UPDATE res_users SET password='', password_crypt=%s WHERE id=%s", (stored_password_crypt, uid))
+                self._set_encrypted_password(cr, uid, stored_password)
         try:
             return super(res_users, self).check_credentials(cr, uid, password)
         except openerp.exceptions.AccessDenied: