Modify

Opened 16 years ago

Closed 16 years ago

#2885 closed defect (fixed)

AccountManager Registration Aborts all transactions

Reported by: Robert M. Zigweid Owned by: Matt Good
Priority: normal Component: AccountManagerPlugin
Severity: blocker Keywords: registration
Cc: Trac Release: 0.11

Description (last modified by John Hampton)

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 (1)

session_store_commit.patch (925 bytes) - added by John Hampton 16 years ago.
Patch to fix missing commit() call in check_password and set_password. Fixes PostgreSQL compatability

Download all attachments as: .zip

Change History (6)

comment:1 Changed 16 years ago by John Hampton

Description: modified (diff)

Fixed SQL log formatting

comment:2 Changed 16 years ago by Robert M. Zigweid

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.

comment:3 Changed 16 years ago by Robert M. Zigweid

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

Changed 16 years ago by John Hampton

Attachment: session_store_commit.patch added

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

comment:4 Changed 16 years ago by John Hampton

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.

comment:5 Changed 16 years ago by John Hampton

Resolution: fixed
Status: newclosed

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

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Matt Good.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.