Xavier (Open ERP) has proposed merging
lp:~openerp-dev/openobject-addons/6.1-crypt-concurrency-xmo into
lp:openobject-addons/6.1.
Requested reviews:
OpenERP Core Team (openerp)
For more details, see:
https://code.launchpad.net/~openerp-dev/openobject-addons/6.1-crypt-concurrency-xmo/+merge/115072
Case id 575984
Issue:
When installing base_crypt from the 6.1 web client, the installation succeeds
(and all passwords are successfully converted) but a traceback appears ending
with:
TransactionRollbackError: could not serialize access due to concurrent
update
Reason:
This is due to a second, concurrent request: after the "install_immediate"
action has returned the web client tries to fetch any new res.log item. Doing
so triggers the `check` method of res.users (overridden by base_crypt) which
delegates to the (also overridden) `_login` method, which updates the user's
access time.
As this is done during the update of the various passwords (for conversion to
hashed form) postgres finds itself with writes in two transactions at the same
time and rejects the second one.
Solution:
Prevent concurrent updates, the global table update (in #init) can just lock
the whole table since it's going to touch every record anyway.
Theoretically, `check` could use a `SELECT FOR UPDATE` to (wait on and) lock
the one row it needs to query and update, but due to the isolation level we use
`SELECT FOR UPDATE` errors out instead[0] (nb: I may have missed something). As
a result, `check` has to lock the table.
[0]
http://www.postgresql.org/docs/8.4/static/transaction-iso.html#XACT-SERIALIZABLE
--
https://code.launchpad.net/~openerp-dev/openobject-addons/6.1-crypt-concurrency-xmo/+merge/115072
Your team OpenERP R&D Team is subscribed to branch
lp:~openerp-dev/openobject-addons/6.1-crypt-concurrency-xmo.
=== modified file 'base_crypt/crypt.py'
--- base_crypt/crypt.py 2012-03-31 01:52:49 +0000
+++ base_crypt/crypt.py 2012-07-16 07:25:34 +0000
@@ -35,9 +35,13 @@
# 59 Temple Place - Suite 330
# Boston, MA 02111-1307
# USA.
+from __future__ import with_statement
+from contextlib import closing
+import logging
from random import seed, sample
from string import ascii_letters, digits
+
from osv import fields,osv
import pooler
from tools.translate import _
@@ -135,6 +139,18 @@
# agi - 022108
# Add handlers for 'input_pw' field.
+ def init(self, cr):
+ with closing(pooler.get_db(cr.dbname).cursor()) as cr:
+ cr.execute('LOCK res_users')
+ cr.execute("SELECT id, password FROM res_users "
+ "WHERE active=true AND password NOT LIKE '$%' ")
+
+ cr.executemany("UPDATE res_users SET password=%s WHERE id=%s",
+ ((encrypt_md5(password, gen_salt()), id)
+ for id, password in cr.fetchall()))
+
+ cr.commit()
+
def set_pw(self, cr, uid, id, name, value, args, context):
if not value:
raise osv.except_osv(_('Error'), _("Please specify the password !"))
@@ -174,17 +190,13 @@
return False
if db is False:
raise RuntimeError("Cannot authenticate to False db!")
- cr = None
+
try:
- cr = pooler.get_db(db).cursor()
- return self._login(cr, db, login, password)
+ with closing(pooler.get_db(db).cursor()) as cr:
+ return self._login(cr, db, login, password)
except Exception:
- import logging
logging.getLogger('netsvc').exception('Could not authenticate')
return Exception('Access Denied')
- finally:
- if cr is not None:
- cr.close()
def _login(self, cr, db, login, password):
cr.execute( 'SELECT password, id FROM res_users WHERE login=%s AND active',
@@ -196,12 +208,6 @@
# Return early if no one has a login name like that.
return False
- stored_pw = self.maybe_encrypt(cr, stored_pw, id)
-
- if not stored_pw:
- # means couldn't encrypt or user is not active!
- return False
-
# Calculate an encrypted password from the user-provided
# password.
obj = pooler.get_pool(db).get('res.users')
@@ -239,8 +245,8 @@
if (cached_pass is not None) and cached_pass == passwd:
return True
- cr = pooler.get_db(db).cursor()
- try:
+ with closing(pooler.get_db(db).cursor()) as cr:
+ cr.execute('LOCK res_users')
if uid not in self._salt_cache.get(db, {}):
# If we don't have cache, we have to repeat the procedure
# through the login function.
@@ -257,8 +263,6 @@
cr.execute('SELECT COUNT(*) FROM res_users WHERE id=%s AND password=%s AND active',
(int(uid), encrypt_md5(passwd, salt)))
res = cr.fetchone()[0]
- finally:
- cr.close()
if not bool(res):
raise security.ExceptionNoTb('AccessDenied')
@@ -270,29 +274,6 @@
else:
self._uid_cache[db] = {uid: passwd}
return bool(res)
-
- def maybe_encrypt(self, cr, pw, id):
- """ Return the password 'pw', making sure it is encrypted.
-
- If the password 'pw' is not encrypted, then encrypt all active passwords
- in the db. Returns the (possibly newly) encrypted password for 'id'.
- """
-
- if not pw.startswith(magic_md5):
- cr.execute("SELECT id, password FROM res_users " \
- "WHERE active=true AND password NOT LIKE '$%'")
- # Note that we skip all passwords like $.., in anticipation for
- # more than md5 magic prefixes.
- res = cr.fetchall()
- for i, p in res:
- encrypted = encrypt_md5(p, gen_salt())
- cr.execute('UPDATE res_users SET password=%s where id=%s',
- (encrypted, i))
- if i == id:
- encrypted_res = encrypted
- cr.commit()
- return encrypted_res
- return pw
users()
# vim:expandtab:smartindent:tabstop=4:softtabstop=4:shiftwidth=4:
_______________________________________________
Mailing list: https://launchpad.net/~openerp-dev-gtk
Post to : [email protected]
Unsubscribe : https://launchpad.net/~openerp-dev-gtk
More help : https://help.launchpad.net/ListHelp
