Ticket #2885 (closed defect: fixed)

Opened 8 months ago

Last modified 8 months ago

AccountManager Registration Aborts all transactions

Reported by: rzigweid Assigned to: mgood
Priority: normal Component: AccountManagerPlugin
Severity: blocker Keywords: registration
Cc: Trac Release: 0.11

Description (Last modified by pacopablo)

I'm trying to set up trac initially and using the AccountManager Plugin to handle my authentication. Snip from trac.ini 

[components]
acct_mgr.* = enabled
trac.web.auth.loginmodule = disabled
webadmin.* = enabled

[account-manager]
hash_method = HtDigestHashMethod
password_store = SessionStore

When submitting the form to register user 'test' I logged the following statements from Postgres. 

LOG:  statement: SET DATESTYLE TO 'ISO'
LOG:  statement: SHOW client_encoding
LOG:  statement: SHOW default_transaction_isolation
LOG:  statement: SET client_encoding = 'UNICODE'
LOG:  statement: BEGIN; SET TRANSACTION ISOLATION LEVEL READ COMMITTED
LOG:  statement: SELECT value FROM system WHERE name='database_version'
LOG:  statement: ABORT
LOG:  statement: BEGIN; SET TRANSACTION ISOLATION LEVEL READ COMMITTED
LOG:  statement: SELECT * FROM session_attribute WHERE authenticated=1 AND name='password' AND sid='test'
LOG:  statement: ABORT
LOG:  statement: BEGIN; SET TRANSACTION ISOLATION LEVEL READ COMMITTED
LOG:  statement: SELECT username,action FROM permission
LOG:  statement: ABORT
LOG:  statement: BEGIN; SET TRANSACTION ISOLATION LEVEL READ COMMITTED
LOG:  statement: SELECT username,action FROM permission
LOG:  statement: ABORT
LOG:  statement: BEGIN; SET TRANSACTION ISOLATION LEVEL READ COMMITTED
LOG:  statement: UPDATE session_attribute SET value=':14d2f8f05e52ae70a58d8e2dd916bbbe' WHERE authenticated=1 AND name='password' AND sid='test'
LOG:  statement: INSERT INTO session_attribute (sid,authenticated,name,value) VALUES ('test',1,'password',':14d2f8f05e52ae70a58d8e2dd916bbbe')
LOG:  statement: ABORT
LOG:  statement: BEGIN; SET TRANSACTION ISOLATION LEVEL READ COMMITTED
LOG:  statement: SELECT count(*) FROM session WHERE sid='test' AND authenticated=1
LOG:  statement: END
LOG:  statement: BEGIN; SET TRANSACTION ISOLATION LEVEL READ COMMITTED
LOG:  statement: SELECT last_visit FROM session WHERE sid='c65b0b1ed8ad25d555b4d2ec' AND authenticated=0
LOG:  statement: ABORT
LOG:  statement: BEGIN; SET TRANSACTION ISOLATION LEVEL READ COMMITTED
LOG:  statement: SELECT username,action FROM permission
LOG:  statement: ABORT
LOG:  statement: BEGIN; SET TRANSACTION ISOLATION LEVEL READ COMMITTED
LOG:  statement: SELECT last_visit FROM session WHERE sid='c65b0b1ed8ad25d555b4d2ec' AND authenticated=0
LOG:  statement: ABORT

Needless to say, the account is not getting created. Why are all the transactions being aborted? Issuing the statements (in particular the INSERT) is successful when doing it from the psql tool logged in as the trac user.

I'm fully prepared to admit the problem exists between keyboard and chair, but I'm not sure what I've done wrong here.

Trac is 0.11b1 AccountManager is from the trunk about April 7 08 PostgreSQL 8.2.7

Attachments

session_store_commit.patch (0.9 kB) - added by pacopablo on 04/14/08 13:02:58.
Patch to fix missing commit() call in check_password and set_password. Fixes PostgreSQL compatability

Change History

04/09/08 14:39:31 changed by pacopablo

  • description changed.

Fixed SQL log formatting

04/09/08 15:33:42 changed by rzigweid

Additional information: My assumption about the ABORTS was incorrect. When I fill in the other entries (name or email), into the form, the rows for those entries are added into the database appropriately, and END for the transaction is appropriately called.

It seems to be something with the password field, though I cannot yet determine what. The same error occurs whether the anonymous registration is used, or registering a user as an admin through the administration interface.

I haven't been able to determine what trac is getting back from PostgreSQL that it doesn't like.

04/14/08 12:20:03 changed by rzigweid

Okay, I found the problem, and I've made a change to db.py/set_password() that seems to work. Ultimately, there's a missing commit. 

    def set_password(self, user, password):
        """Sets the password for the user.  This should create the user account
        if it doesn't already exist.
        Returns True if a new account was created, False if an existing account
        was updated.
        """
        hash = self.hash_method.generate_hash(user, password)
        db = self.env.get_db_cnx()
        cursor = db.cursor()
        cursor.execute("UPDATE session_attribute "
                       "SET value=%s "
                       "WHERE authenticated=1 AND name='password' "
                       "AND sid=%s", (hash, user))
        if cursor.rowcount > 0:
            return False # updated existing password
        cursor.execute("INSERT INTO session_attribute "
                       "(sid,authenticated,name,value) "
                       "VALUES (%s,1,'password',%s)",
                       (user, hash))
        if cursor.rowcount == 1:
           db.commit()
           return True
        else:
           # XXX: There should be a message here as to why this failed.
           return False

04/14/08 13:02:58 changed by pacopablo

  • attachment session_store_commit.patch added.

Patch to fix missing commit() call in check_password and set_password. Fixes PostgreSQL compatability

04/14/08 13:06:10 changed by pacopablo

I had been running a patched version of account manager and was looking at my patched code, so I couldn't see how it was failing. However, I attached the patch that I've been using.

This is semi-related to #1484 in the sense that that patch fixes these issues also.

04/14/08 16:46:08 changed by pacopablo

  • status changed from new to closed.
  • resolution set to fixed.

(In [3515]) Added explicit commit() calls to keep the transaction from aborting. Fixes #2885

Add/Change #2885 (AccountManager Registration Aborts all transactions)




Change Properties
Action