Modify

Opened 11 years ago

Closed 10 years ago

Last modified 10 years ago

#11558 closed defect (fixed)

LdapAuthStore not storing email address in session_attribute from ldap

Reported by: anonymous Owned by: igoltz
Priority: highest Component: LdapAuthStorePlugin
Severity: critical Keywords:
Cc: Trac Release:

Description (last modified by Ryan J Ollos)

The plugin LdapAuthStore works for the login against ldap, but does not seeem to be storing the email address from ldap. Indeed after the succesful login, in the database the session_attribute table is empty.

This is a problem because the AnnouncerPlugin logs as follows

Trac[mail] DEBUG: EmailDistributor was unable to find an address for:

an no mail is sent for notification.

The trac.ini is configured like the example in LdapAuthStorePlugin

and the values are aligned to ldap, like this

[account-manager]
password_store = LdapAuthStore
email_attr = mail
name_attr = cn

How to troubleshoot this? The trac log is enabled at debug level, but the LdapAuthStorePlugin seems silent, how to make it more verbose?

Attachments (2)

3668f9df2b4083370381bd63f5e4e91e021f3b49.tgz (4.5 KB) - added by igoltz 11 years ago.
git commit 3668f9df2b4083370381bd63f5e4e91e021f3b49 Wed Sep 11 11:11:28 2013 +0200
t11558.patch (15.6 KB) - added by Ryan J Ollos 11 years ago.

Download all attachments as: .zip

Change History (32)

comment:1 Changed 11 years ago by anonymous

using

Trac 0.12.5
LdapAuthStorePlugin 0.3.0
LdapPlugin 0.7.0dev
TracAccountManager 0.4.3
TracLDAPAuth 1.2.1
Last edited 11 years ago by Ryan J Ollos (previous) (diff)

comment:2 Changed 11 years ago by anonymous

From comment:1 disabled TracLDAPAuth 1.2.1 still the issue: login works but the email address of users are not stored in session_attribute.

comment:3 Changed 11 years ago by anonymous

Tried to add manually an entry into session_attribute (sid|authenticated|name|value) with did set to login name, authenticated set to 1, name set to 'email' and value set with an address. AnnouncerPlugin works and sends the notification email to the address.

So the problem is isolated: LdapAuthStorePlugin and LdapPlugin do authenticate against ldap, but do not store email attribute from ldap into session_attribute.

Please let me know what can be done. This is urging and I am wondering how long is it needed to fix this issue? days? weeks? months? years?

comment:4 Changed 11 years ago by anonymous

I have not tried it, but AccountLdapPlugin sounds like it has the functionality you require. It says "moved the basic properties of LDAP (user and mail) to the corresponding properties in Trac".

comment:5 Changed 11 years ago by Ryan J Ollos

Description: modified (diff)

comment:6 Changed 11 years ago by igoltz

Can't reproduce, it stores email an name in the session_attribute! Please check your config with LdapAuthStorePlugin. There is no such key email_attr, all ldap config goes under [ldap]. And i run Trac 1.0.1

Btw., this plugin does not handle non ascii characters in names. Or better, ldap plugin does not, wich is used. I did some changes to have it working but fear to commit as this may break the old behavior. After working some time with the plugin it seems to me its better to rewrite, abandon the ldap plugin base, or combine with DirectoryAuth plugin efforts.

I could provide the version i have running as tgz.

Last edited 11 years ago by Ryan J Ollos (previous) (diff)

comment:7 in reply to:  6 Changed 11 years ago by Ryan J Ollos

Replying to igoltz:

... or combine with DirectoryAuth plugin efforts.

This would be great! DirectoryAuthPlugin is really in need of some care. The previous maintainer is not very responsive, so if you open a ticket and ask for commit access, and there is no response in two weeks, then I'll add you as an additional maintainer for the project (per the guidelines at AdoptingHacks).

comment:8 Changed 11 years ago by Ryan J Ollos

Another thing to consider ... are you guys using different database backends? SQLite is very forgiving, but PostgreSQL much more strict. You may need to quote the literal 1 for cross-db compatibility (add single-quotes around literal): ldapauthstoreplugin/trunk/ldapauthstore/ldap_store.py@13347:141-142,149-151#L116.

To help debug, you could set [trac] debug_sql = True (see TracIni#trac-section for more details).

Last edited 11 years ago by Ryan J Ollos (previous) (diff)

comment:9 Changed 11 years ago by anonymous

It's great to read your feedback! :)

Indeed there's no email_attr, it was placed for another plugin DirectoryAuthPlugin that is for now discontinued in favor of LdapAuthStore because it works for login against openldap (it would be nicer if possible also with non ascii chars too). Don't know what combination would work best together between LdapAuthStorePlugin, LdapPlugin, TracLDAPAuth, LdapAuthPlugin, DirectoryAuthPlugin.

In the trac.ini there is

[ldap]
name = cn
email = mail

I have enabled the debug_sql (thanks for pointing that!) and found that during the login (against ldap) the first sql is an update to session_attribute

'UPDATE session_attribute SET value=%s WHERE name=%s AND sid=%s AND authenticated=1'

fine, but it is not followed by an insert for when the record to update is not already present. An update does not insert a new record. The database is SQLite3.

Which module is supposed to populate the session_attribute (insert) from ldap? Please try deleting session_attribute and do a login, what happens on your side?

Best regards :) Nik

comment:10 Changed 11 years ago by igoltz

I have mysql as backend and I remember there was something... The version as found here and patched together looks like this. If the update fails there should be an insert.

                cursor.execute("SELECT * from session_attribute WHERE sid=%s", (user))
                db.commit()
                if not cursor.rowcount:
                    cursor.execute("UPDATE session_attribute SET value=%s " 
                                   "WHERE name=%s AND sid=%s AND authenticated=1", 
                                   (value, attr, user)) 
                    db.commit()
                    if not cursor.rowcount:
                        cursor.execute("INSERT INTO session_attribute " 
                                       "(sid,authenticated,name,value) " 
                                       "VALUES (%s,1,%s,%s)", 
                                       (user, attr, value)) 
                        db.commit() 

It did not work, version 0.3.0 tagged in the svn has on line more.

			db.commit()
			cursor.execute("SELECT * from session_attribute WHERE name=%s AND sid=%s", (attr,user))
			db.commit()

Maybe you got the /0.11 code. The actual is in /trunk as i work with trac 1.0.1. Please check this.

comment:11 Changed 11 years ago by anonymous

On debian did:

sudo apt-get install sqlite3
sudo apt-get install trac 
sudo apt-get install python-ldap

Unzipped

ldapauthstoreplugin-13648.zip

entered the trunk folder and did a

python setup.py bdist_egg

and copied the .egg file from dist folder to the trac project's plugin folder.

LdapAuthStorePlugin-0.3.0-py2.7.egg

About the trac version: I'm using 12.5 because it's the version provided by apt-get of debian stable 7.3. (A small dilemma: it's better to install trac via apt-get and possibly get easily security updates -in debian 7.3 trac versionis 12.5-, or to install manually the latest trac (>1.x) and not get automatic security fixes?)

In the trunk folder i see this code:

        def check_password(self, user, password):
                """Checks if the password is valid for the user.

                Does so by attempting an LDAP bind as this user.
                Does not attempt to check this is a valid trac account,
                see has_user for that.
                """
                dn = self.util.user_attrdn(user)
                cnx = self._bind_as_user(dn, password)
                if cnx is None:
                        return False
                """Store values from ldap in the session cache
                or update if values in ldap changed
                """
                for attr in ('name', 'email'):
                        fieldname = str(self.config.get('ldap', attr ))
                        self.env.log.debug('LDAPstore : Getting %s for %s' % ( fieldname , attr ))
                        value = cnx.get_attribute(dn, fieldname)
                        if not value:
                                continue
                        value = unicode(value[0], 'utf-8')
                        self.env.log.debug('LDAPstore : Got value %s for attribute %s' % ( value , attr ))
                        db = self.env.get_db_cnx()
                        cursor = db.cursor()
                        self.env.log.debug('LDAPstore : Update %s for %s' % ( value , attr ))
                        cursor.execute("UPDATE session_attribute SET value=%s "
                                "WHERE name=%s AND sid=%s AND authenticated=1",
                                (value, attr, user))
                        db.commit()
                        cursor.execute("SELECT * from session_attribute WHERE name=%s AND sid=%s", (attr,user))
                        db.commit()
                        if not cursor.rowcount:
                                self.env.log.debug('LDAPstore : Insert %s for %s' % ( value , attr ))
                                cursor.execute("INSERT INTO session_attribute "
                                        "(sid,authenticated,name,value) "
                                        "VALUES (%s,1,%s,%s)",
                                        (user, attr, value))
                                db.commit()
                cnx.close()

Here's the log about trying a login with a new user:

2014-02-12 12:35:26,609 Trac[main] DEBUG: Dispatching <Request "POST '/login'">
2014-02-12 12:35:26,609 Trac[web_ui] DEBUG: LoginModule._remote_user: Authentication attempted for 'test user 1'
2014-02-12 12:35:26,665 Trac[ldap_store] DEBUG: LDAPstore : Getting cn for name
2014-02-12 12:35:26,666 Trac[ldap_store] DEBUG: LDAPstore : Got value test user 1 for attribute name
2014-02-12 12:35:26,666 Trac[ldap_store] DEBUG: LDAPstore : Update test user 1 for name
2014-02-12 12:35:26,667 Trac[util] DEBUG: SQL: 'UPDATE session_attribute SET value=%s WHERE name=%s AND sid=%s AND authenticated=1'
2014-02-12 12:35:26,667 Trac[util] DEBUG: args: (u'test user 1', 'name', u'test user 1')
2014-02-12 12:35:26,667 Trac[util] DEBUG: prefetch: 0 rows
2014-02-12 12:35:26,668 Trac[util] DEBUG: SQL: 'SELECT * from session_attribute WHERE name=%s AND sid=%s'
2014-02-12 12:35:26,668 Trac[util] DEBUG: args: ('name', u'test user 1')
2014-02-12 12:35:26,668 Trac[util] DEBUG: prefetch: 0 rows
2014-02-12 12:35:26,669 Trac[ldap_store] DEBUG: LDAPstore : Getting mail for email
2014-02-12 12:35:26,670 Trac[ldap_store] DEBUG: LDAPstore : Got value test_user_1@localhost.net for attribute email
2014-02-12 12:35:26,670 Trac[ldap_store] DEBUG: LDAPstore : Update test_user_1@localhost.net for email
2014-02-12 12:35:26,670 Trac[util] DEBUG: SQL: 'UPDATE session_attribute SET value=%s WHERE name=%s AND sid=%s AND authenticated=1'
2014-02-12 12:35:26,671 Trac[util] DEBUG: args: (u'test_user_1@localhost.net', 'email', u'test user 1')
2014-02-12 12:35:26,671 Trac[util] DEBUG: prefetch: 0 rows
2014-02-12 12:35:26,671 Trac[util] DEBUG: SQL: 'SELECT * from session_attribute WHERE name=%s AND sid=%s'
2014-02-12 12:35:26,672 Trac[util] DEBUG: args: ('email', u'test user 1')
2014-02-12 12:35:26,672 Trac[util] DEBUG: prefetch: 0 rows
2014-02-12 12:35:26,673 Trac[util] DEBUG: SQL: '\n            SELECT  *\n            FROM    session_attribute\n            \n            WHERE   authenticated=1\n                AND name=%s\n                AND sid=%s\n            '
2014-02-12 12:35:26,673 Trac[util] DEBUG: args: ('password_reset', u'test user 1')
2014-02-12 12:35:26,673 Trac[util] DEBUG: prefetch: 0 rows
2014-02-12 12:35:26,674 Trac[web_ui] DEBUG: LoginModule.authenticate: Set 'REMOTE_USER' = 'test user 1'

the new user does not result in session_attribute.

Changed 11 years ago by igoltz

git commit 3668f9df2b4083370381bd63f5e4e91e021f3b49 Wed Sep 11 11:11:28 2013 +0200

comment:12 Changed 11 years ago by igoltz

If the select after the update fails (as it does: prefetch 0 rows) then the insert statement should execute.

Please check with attachment git commit 3668f9df2b4083370381bd63f5e4e91e021f3b49 Wed Sep 11 11:11:28 2013 +0200

comment:13 in reply to:  11 Changed 11 years ago by Ryan J Ollos

Replying to anonymous:

About the trac version: I'm using 12.5 because it's the version provided by apt-get of debian stable 7.3. (A small dilemma: it's better to install trac via apt-get and possibly get easily security updates -in debian 7.3 trac versionis 12.5-, or to install manually the latest trac (>1.x) and not get automatic security fixes?)

It really just depends how comfortable you are installing Trac and how much effort you want to put into maintaining it. It's really not that much effort though, as minor versions of Trac are release no more than every 3 months (and it's been close to a year since the last minor release). Trac 1.0 is certainly nicer looking and has a lot more features than 0.12.

comment:14 Changed 11 years ago by anonymous

Thank you rjollos for the patch. It is much improved from the log aspect, I think this is very important to help solve quickly criptic situations and thus ease its adoption. Great :)

I have removed the previous LdapAuthStorePlugin-0.3.0-py2.7.egg plugin and replaced with the new egg from the compilation of untarred 3668f9df2b4083370381bd63f5e4e91e021f3b49.tgz, then restarted.

The login does not work, so I updated the trac.ini with the following values, I report those that are used by ldap_store (by reading the ldap_store.py code):

[ldap]
# only values used by ldap_store
allusers_group = enabled_trac_team_group
basedn = dc=my_site,dc=net
email = mail
groupattr = cn
groupmember = member
groupmemberisdn = yes
group_filter = (objectClass=groupOfNames)
name = cn
scope = 2
uidattr = cn
user_filter = (objectClass=person)

Now with the new LdapAuthStorePlugin the login works, but unfortunately the new user is still not inserted into session_attribute. Here's the log:

2014-02-13 10:53:55,096 Trac[main] DEBUG: Dispatching <Request "POST '/login'">
2014-02-13 10:53:55,097 Trac[web_ui] DEBUG: LoginModule._remote_user: Authentication attempted for 'test user 1'
2014-02-13 10:53:55,098 Trac[ldap_store] DEBUG: check_password(test user 1, XXXXXXXX)
2014-02-13 10:53:55,098 Trac[ldap_store] DEBUG: Search for: dc=my_site,dc=net (&(cn=test user 1)(objectClass=person)) ['dn', 'cn', 'mail', 'cn'] 2
2014-02-13 10:53:55,100 Trac[ldap_store] DEBUG: User test user 1 is spelled test user 1 and dn is cn=test user 1,ou=people,dc=my_site,dc=net
2014-02-13 10:53:55,153 Trac[ldap_store] DEBUG: Check if user is in valid group
2014-02-13 10:53:55,153 Trac[ldap_store] DEBUG: has_user (test user 1)
2014-02-13 10:53:55,153 Trac[ldap_store] DEBUG: _get_user_dn(test user 1)
2014-02-13 10:53:55,154 Trac[ldap_store] DEBUG: Search for: dc=my_site,dc=net (&(cn=test user 1)(objectClass=person)) ['dn'] 2
2014-02-13 10:53:55,154 Trac[ldap_store] DEBUG: User test user 1 has dn cn=test user 1,ou=people,dc=my_site,dc=net
2014-02-13 10:53:55,155 Trac[ldap_store] DEBUG: _get_group_dn(enabled_trac_team_group)
2014-02-13 10:53:55,155 Trac[ldap_store] DEBUG: Search for: dc=my_site,dc=net (&(cn=enabled_trac_team_group)(objectClass=groupOfNames)) ['dn'] 2
2014-02-13 10:53:55,156 Trac[ldap_store] DEBUG: Group enabled_trac_team_group has dn cn=enabled_trac_team_group,ou=groups,dc=my_site,dc=net
2014-02-13 10:53:55,157 Trac[ldap_store] DEBUG: LDAPstore : Update test user 1 for name
2014-02-13 10:53:55,157 Trac[util] DEBUG: SQL: 'UPDATE session_attribute SET value=%s WHERE name=%s AND sid=%s AND authenticated=1'
2014-02-13 10:53:55,157 Trac[util] DEBUG: args: (u'test user 1', 'name', 'test user 1')
2014-02-13 10:53:55,158 Trac[util] DEBUG: prefetch: 0 rows
2014-02-13 10:53:55,158 Trac[util] DEBUG: SQL: 'SELECT * from session_attribute WHERE name=%s AND sid=%s'
2014-02-13 10:53:55,158 Trac[util] DEBUG: args: ('name', 'test user 1')
2014-02-13 10:53:55,159 Trac[util] DEBUG: prefetch: 0 rows
2014-02-13 10:53:55,159 Trac[ldap_store] DEBUG: LDAPstore : Update test_user_1@localhost.net for email
2014-02-13 10:53:55,159 Trac[util] DEBUG: SQL: 'UPDATE session_attribute SET value=%s WHERE name=%s AND sid=%s AND authenticated=1'
2014-02-13 10:53:55,160 Trac[util] DEBUG: args: (u'test_user_1@localhost.net', 'email', 'test user 1')
2014-02-13 10:53:55,160 Trac[util] DEBUG: prefetch: 0 rows
2014-02-13 10:53:55,160 Trac[util] DEBUG: SQL: 'SELECT * from session_attribute WHERE name=%s AND sid=%s'
2014-02-13 10:53:55,160 Trac[util] DEBUG: args: ('email', 'test user 1')
2014-02-13 10:53:55,161 Trac[util] DEBUG: prefetch: 0 rows
2014-02-13 10:53:55,161 Trac[web_ui] DEBUG: LoginModule.authenticate: Set 'REMOTE_USER' = 'test user 1'
2014-02-13 10:53:55,162 Trac[util] DEBUG: SQL: 'DELETE FROM auth_cookie WHERE time < %s'
2014-02-13 10:53:55,162 Trac[util] DEBUG: args: (1391421235,)
2014-02-13 10:53:55,163 Trac[util] DEBUG: prefetch: 0 rows
2014-02-13 10:53:55,163 Trac[util] DEBUG: SQL: 'INSERT INTO auth_cookie (cookie,name,ipnr,time) VALUES (%s, %s, %s, %s)'
2014-02-13 10:53:55,163 Trac[util] DEBUG: args: ('f56e1b449b03515313f19ec28bbff17c', u'test user 1', '192.168.3.50', 1392285235)
2014-02-13 10:53:55,164 Trac[util] DEBUG: prefetch: 0 rows
2014-02-13 10:53:55,230 Trac[util] DEBUG: SQL: '\n                SELECT authenticated FROM session WHERE sid=%s OR sid=%s\n                '
2014-02-13 10:53:55,231 Trac[util] DEBUG: args: ('b178ea4d41ded4cdb0e3301a', u'test user 1')
2014-02-13 10:53:55,231 Trac[util] DEBUG: prefetch: 1 rows
2014-02-13 10:53:55,231 Trac[session] DEBUG: Retrieving session for ID u'test user 1'
2014-02-13 10:53:55,232 Trac[util] DEBUG: SQL: '\n            SELECT last_visit FROM session WHERE sid=%s AND authenticated=%s\n            '
2014-02-13 10:53:55,232 Trac[util] DEBUG: args: (u'test user 1', 1)
2014-02-13 10:53:55,232 Trac[util] DEBUG: prefetch: 1 rows
2014-02-13 10:53:55,233 Trac[util] DEBUG: SQL: '\n            SELECT name,value FROM session_attribute\n            WHERE sid=%s and authenticated=%s\n            '
2014-02-13 10:53:55,233 Trac[util] DEBUG: args: (u'test user 1', 1)
2014-02-13 10:53:55,233 Trac[util] DEBUG: prefetch: 0 rows
2014-02-13 10:53:55,356 Trac[main] DEBUG: Dispatching <Request "GET '/admin/general/plugin'">
2014-02-13 10:53:55,359 Trac[util] DEBUG: SQL: 'SELECT name FROM auth_cookie WHERE cookie=%s'
2014-02-13 10:53:55,359 Trac[util] DEBUG: args: ('f56e1b449b03515313f19ec28bbff17c',)
2014-02-13 10:53:55,359 Trac[util] DEBUG: prefetch: 1 rows
2014-02-13 10:53:55,360 Trac[session] DEBUG: Retrieving session for ID u'test user 1'
2014-02-13 10:53:55,360 Trac[util] DEBUG: SQL: '\n            SELECT last_visit FROM session WHERE sid=%s AND authenticated=%s\n            '
2014-02-13 10:53:55,360 Trac[util] DEBUG: args: (u'test user 1', 1)
2014-02-13 10:53:55,361 Trac[util] DEBUG: prefetch: 1 rows
2014-02-13 10:53:55,361 Trac[util] DEBUG: SQL: '\n            SELECT name,value FROM session_attribute\n            WHERE sid=%s and authenticated=%s\n            '
2014-02-13 10:53:55,361 Trac[util] DEBUG: args: (u'test user 1', 1)
2014-02-13 10:53:55,362 Trac[util] DEBUG: prefetch: 0 rows
2014-02-13 10:53:55,362 Trac[util] DEBUG: SQL: 'SELECT name FROM auth_cookie WHERE cookie=%s'
2014-02-13 10:53:55,362 Trac[util] DEBUG: args: ('f56e1b449b03515313f19ec28bbff17c',)
2014-02-13 10:53:55,363 Trac[util] DEBUG: prefetch: 1 rows
2014-02-13 10:53:55,366 Trac[api] DEBUG: groups: @enabled_trac_team_group,@trac-enabled_trac_team_group
2014-02-13 10:53:55,366 Trac[ldap_store] DEBUG: _get_user_dn(test user 1)
2014-02-13 10:53:55,367 Trac[ldap_store] DEBUG: Search for: dc=my_site,dc=net (&(cn=test user 1)(objectClass=person)) ['dn'] 2
2014-02-13 10:53:55,368 Trac[ldap_store] DEBUG: User test user 1 has dn cn=test user 1,ou=people,dc=my_site,dc=net
2014-02-13 10:53:55,368 Trac[ldap_store] DEBUG: Search for: dc=my_site,dc=net (&(member=cn=test user 1,ou=people,dc=my_site,dc=net)(objectClass=groupOfNames)) ['cn'] 2
2014-02-13 10:53:55,369 Trac[ldap_store] DEBUG: user test user 1 has groups @enabled_trac_team_group, @trac-enabled_trac_team_group

Are all fields (like login name, password, email) properly escaped and apostrophed?

What is preventing the insertion in SQLite3?

Please advise on what would be the best plugin combination for ldap authentication (possibly also in utf-8) and user attributes store from ldap (possibly specified in trac.ini which attributes to copy), in order to avoid overlapping/conflicting plugins. If I suppose correctly LdapAuthStorePlugin needs LdapPlugin, TracAccountManager and nothing else?

I will try to install trac > 1 version, please advice what would be the best way to install it (easy_install, pip, ...).

Thanks and regards, Nik

comment:15 in reply to:  14 ; Changed 11 years ago by Ryan J Ollos

Replying to anonymous:

Thank you rjollos for the patch.

The patch was actually from igoltz.

comment:16 in reply to:  15 Changed 11 years ago by anonymous

Replying to rjollos:

The patch was actually from igoltz.

Apologize me! Thank you igoltz for the patch :) and everyone for the involvment.

Seems something is not working here

cursor.execute("SELECT * from session_attribute WHERE name=%s AND sid=%s", (attr, user_attr))
db.commit()
if not cursor.rowcount:
    self.log.debug('LDAPstore : Insert %s for %s' % ( value , attr ))

any idea on what could be?

Nik

comment:18 Changed 11 years ago by anonymous

Nik, I cant spend time on this for the next weeks, sorry. But it looks like you alreaady found the (sqlite specific) error. Could you try something like if not cursor.rowcount or cursor.rowcount == -1: please.

comment:19 Changed 11 years ago by Ryan J Ollos

The method rowcount isn't defined in the Trac database API, so I imagine it's not cross-DB compatible.

Usually we use row = cursor.fetchone() or rows = cursor.fetchall() and then test:

if rows[0]:
    # do something

Also, you can remove the db.commit() after the SELECT. I'll prepare an untested patch and post it here.

Finally, the indentation of the file is messed up, so I'm going to run reindent.py on it. I hope that is okay!

comment:20 Changed 11 years ago by Ryan J Ollos

In 13662:

Fixed indentation using reindent.py. Refs #11558.

comment:21 Changed 11 years ago by Ryan J Ollos

In 13663:

More indentation fixes. Refs #11558.

comment:22 Changed 11 years ago by Ryan J Ollos

Untested patch:

  • ldapauthstoreplugin/trunk/ldapauthstore/ldap_store.py

    diff --git a/ldapauthstoreplugin/trunk/ldapauthstore/ldap_store.py b/ldapauthstoreplugin/trunk/ldapauthst
    index 4b2eaaa..c4a14f8 100644
    a b class LdapAuthStore(Component): 
    138138            cursor = db.cursor()
    139139            self.env.log.debug('LDAPstore : Update %s for %s' % ( value , attr ))
    140140            cursor.execute("UPDATE session_attribute SET value=%s "
    141                     "WHERE name=%s AND sid=%s AND authenticated=1",
     141                    "WHERE name=%s AND sid=%s AND authenticated='1'",
    142142                    (value, attr, user))
    143143            db.commit()
    144144            cursor.execute("SELECT * from session_attribute WHERE name=%s AND sid=%s", (attr,user))
    145             db.commit()
    146             if not cursor.rowcount:
     145            if not cursor.fetchone():
    147146                self.env.log.debug('LDAPstore : Insert %s for %s' % ( value , attr ))
    148147                cursor.execute("INSERT INTO session_attribute "
    149148                        "(sid,authenticated,name,value) "

I didn't look at igoltz's changes from comment:12, so you may want to merge those into the patch.

comment:23 Changed 11 years ago by anonymous

I applied the patch from comment:18, this works in storing the new users' email attribute in empty session_attribute, but has a problem when the same user logs out and then tries to login again. An error is displayed on the browser:

Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/trac/web/api.py", line 458, in send_error
    data, 'text/html')
  File "/usr/lib/python2.7/dist-packages/trac/web/chrome.py", line 831, in render_template
    message = req.session.pop('chrome.%s.%d' % (type_, i))
  File "/usr/lib/python2.7/dist-packages/trac/web/api.py", line 228, in __getattr__
    value = self.callbacks[name](self)
  File "/usr/lib/python2.7/dist-packages/trac/web/main.py", line 306, in _get_session
    return Session(self.env, req)
  File "/usr/lib/python2.7/dist-packages/trac/web/session.py", line 213, in __init__
    if req.authname == 'anonymous':
  File "/usr/lib/python2.7/dist-packages/trac/web/api.py", line 228, in __getattr__
    value = self.callbacks[name](self)
  File "/usr/lib/python2.7/dist-packages/trac/web/main.py", line 165, in authenticate
    authname = authenticator.authenticate(req)
  File "build/bdist.linux-x86_64/egg/acct_mgr/util.py", line 82, in wrap
    return func(self, *args, **kwds)
  File "build/bdist.linux-x86_64/egg/acct_mgr/web_ui.py", line 338, in authenticate
    user = self._remote_user(req)
  File "build/bdist.linux-x86_64/egg/acct_mgr/web_ui.py", line 684, in _remote_user
    if acctmgr.check_password(user, password) == True:
  File "build/bdist.linux-x86_64/egg/acct_mgr/api.py", line 259, in check_password
    valid = store.check_password(user, password)
  File "build/bdist.linux-x86_64/egg/ldapauthstore/ldap_store.py", line 261, in check_password
    (user_attr, attr, value))
  File "/usr/lib/python2.7/dist-packages/trac/db/util.py", line 54, in execute
    r = self.cursor.execute(sql_escape_percent(sql), args)
  File "/usr/lib/python2.7/dist-packages/trac/db/sqlite_backend.py", line 78, in execute
    result = PyFormatCursor.execute(self, *args)
  File "/usr/lib/python2.7/dist-packages/trac/db/sqlite_backend.py", line 56, in execute
    args or [])
  File "/usr/lib/python2.7/dist-packages/trac/db/sqlite_backend.py", line 48, in _rollback_on_error
    return function(self, *args, **kwargs)
IntegrityError: columns sid, authenticated, name are not unique

comment:24 Changed 11 years ago by anonymous

I applied the patch from comment:22 on the code from comment:12 on trac 12.5 (debian 7.3 apt-get distribution) and it works:

  • success good login
  • success logout and login
  • success no login with bad password
  • success email change from ldap at login

So this should be good for SQLite3:

if not cursor.fetchone(): 

Nik

comment:25 Changed 11 years ago by anonymous

Please someone could pull the tested patch (comment:24) into the source? Thanks

comment:26 Changed 11 years ago by anonymous

About comment:6 and comment:7, indeed the code (LdapAuthStorePlugin + LdapPlugin) does not handle non ascii chars, like accented chars:

  File "/usr/lib/python2.7/dist-packages/ldap/ldapobject.py", line 99, in _ldap_call
    result = func(*args,**kwargs)
UnicodeEncodeError: 'ascii' codec can't encode character u'\xf2' in position 10: ordinal not in range(128)

but it's another story.

In case with DirectoryAuthPlugin it would be possible to handle utf-8, please make that clear in the documentation. So if someone wants to enable ldap in trac, knows what plugins to install amongst so many. Who knows clearly that LdapAuthStorePlugin works well with LdapPlugin or DirectoryAuthPlugin?

Best regards, Nik

Changed 11 years ago by Ryan J Ollos

Attachment: t11558.patch added

comment:27 Changed 11 years ago by Ryan J Ollos

The diff of the code from comment:12 against a revision before r13662 is: attachment:t11558.patch. I think it would be best if igoltz ran a reindent.py on the git branch to fix the indentation, applied the patch from comment:22, and then pushed all those changes to SVN repository. Just a suggestion though. Glad it is working now!

comment:28 Changed 11 years ago by Ryan J Ollos

Btw, for future reference this page can be really useful for hints on how to utilize the Trac DB API: trac:TracDev/DatabaseApi.

comment:29 Changed 10 years ago by igoltz

Resolution: fixed
Status: newclosed

In 14106:

Integrated some changes to support utf8 chars. Better AD support. And fixes #11558

comment:30 Changed 10 years ago by igoltz

commit 14106 is what rjollos suggested in comment:27

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain igoltz.
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.