Ticket #9252 (closed defect: fixed)

Opened 2 years ago

Last modified 6 months ago

All session attributes are deleted when user logs in first time

Reported by: stoecker Assigned to: hasienda
Priority: high Component: AccountManagerPlugin
Severity: normal Keywords: user creation
Cc: rjollos, stuge Trac Release: 0.12

Description

When a new users is created and "verify_email" is false, the entry in "session" is defined as not authenticated: 

select * from session where sid == 'zzzt';
zzzt|0|0

When user logs in, the data for username and email is no longer there. When data is entered again, then session structure gets a new entry, which seems dangerous to me: 

select * from session where sid == 'zzzt';
zzzt|0|0
zzzt|1|1318262166

Afterwards it seems to work, but I expect trouble.

Issues: - After login username/mail data gets lots (but is in database) - After changes there are two entries in session structure, which may break dataset (sid should be unique).

I get broken database from time to time and I assume this may be the reason for this.

Installed version: TracAccountManager?-0.4dev_r10747

Attachments

Change History

(follow-up: ↓ 5 ) 10/10/11 18:16:14 changed by stoecker

NOTE: When I create a user as admin, authenticated is 0 as well.

(follow-up: ↓ 4 ) 10/10/11 18:19:51 changed by stoecker

Also the reset-password function sends me an email with new password, but valid is still the old password. Don't know if this is related or another issue.

10/10/11 18:20:24 changed by stoecker

I use HtPasswd? password store BTW.

(in reply to: ↑ 2 ) 10/11/11 21:56:30 changed by hasienda

  • keywords set to user creation.

Replying to stoecker:

Also the reset-password function sends me an email with new password, but valid is still the old password. Don't know if this is related or another issue.

Not exactly right. This is a new feature added in [10264], to take care of complains about possible dangerous behavior according to #816. You'll see that both passwords are valid before the next successful login. No matter what are your primary password stores, the (alternative) new password is stored in a separate db-based store ResetPwStore, that is derived from SessionStore.

(in reply to: ↑ 1 ; follow-up: ↓ 14 ) 10/11/11 22:12:49 changed by hasienda

Replying to stoecker:

NOTE: When I create a user as admin, authenticated is 0 as well.

Do you suggest, that we should lie about that again? In fact authenticated has been set before [10520], but starting with [10585] it's not authenticated, until the user has been really authenticated at least once.

I'll have to find a way to preserve the initial attributes. There's obviously something wrong in the way the unauthenticated session is propagated into an authenticated one.

(in reply to: ↑ description ) 10/11/11 22:21:56 changed by hasienda

  • priority changed from highest to high.

Replying to stoecker:

When user logs in, the data for username and email is no longer there. When data is entered again, then session structure gets a new entry, which seems dangerous to me: Afterwards it seems to work, but I expect trouble.

Why? Don't think so. At least the db schema suggests, that only sid and authenticate together are required to be unique. I can reproduce the second session, that results in two entries in session table. Really annoying issue, but highest priority?

I get broken database from time to time and I assume this may be the reason for this.

I'd really appreciate to give more help, if possible. Could you provide such a broken db, at least with the session and session_attribute table, other tables and sensitive content stripped?

(follow-up: ↓ 8 ) 10/12/11 08:10:18 changed by anonymous

Hello,

I'd really appreciate to give more help, if possible. Could you provide such a broken db, at least with the session and session_attribute table, other tables and sensitive content stripped?

I don't think so. But according to your text I assume there is another reason.

separate db-based store ResetPwStore?, that is derived from SessionStore?

Shouldn't there be another table in the trac DB then?

(in reply to: ↑ 7 ) 10/12/11 23:23:12 changed by hasienda

Replying to anonymous:

Hello,

I'd really appreciate to give more help, if possible. Could you provide such a broken db, at least with the session and session_attribute table, other tables and sensitive content stripped?

I don't think so. But according to your text I assume there is another reason.

Just an offer.

separate db-based store ResetPwStore, that is derived from SessionStore

Shouldn't there be another table in the trac DB then?

No, why? SessionStore is yet another content welded into session_attribute where the attribute name is the key. Same technique as ticket_custom. And you can easily have multiple SessionStore instances in that table (I changed the class that way), just use another name than 'password' for self.key: ResetPwStore's key is 'password_reset' - as can be read at the top of changeset [10264].

Actually I can't believe that I explain this to you. Sorry, but did you care to read my comments and corresponding links? I'm certainly far less experienced than you, but OTOH I don't do random programming for pure chance. Bear with me.

(follow-up: ↓ 10 ) 10/13/11 13:05:11 changed by stoecker

Actually I can't believe that I explain this to you.

I'm in no way a Trac expert.

key is 'password_reset'

I now checked it and I get a password change e-mail. A new entry is added to DB: 

zzz|1|password_reset|:e7ba839ea782bdf77dae2a64a79f89ca

But I can't use the password for login. Only the old password works. So either password retrieval from the DB or password check fail.

You can try yourself if you want: Username is zzz at josm.openstreetmap.de. Old password is "zzz". New one is "ffku4oxw".

JOSM has open account policy, so you can change the account like you wish :-)

Regarding the authenticated issue. For a new entered user the entry is like this: 

session:
zzz2|0|0
session_attribute:
zzz2|1|email|zzz2@dstoecker.de
zzz2|1|name|zzz2

So while authenticated is "0", the session entry is "1". This does not match. Maybe this is the reason why the values are lost?

(in reply to: ↑ 9 ) 10/14/11 00:14:42 changed by hasienda

  • status changed from new to assigned.

Replying to stoecker:

Actually I can't believe that I explain this to you.

I'm in no way a Trac expert.

Well, only I thought so. :-)

key is 'password_reset'

I now checked it and I get a password change e-mail. A new entry is added to DB: [...] But I can't use the password for login. Only the old password works. So either password retrieval from the DB or password check fail.

I've tested the procedure several times during it's development. But I can't deny the possibility of later modifications, that broke it again. (More and more I see the big value if test suites here, once you have them.)

You can try yourself if you want: Username is zzz [...] JOSM has open account policy, so you can change the account like you wish :-)

Oh, that's an unexpected offer. I'll check.

Regarding the authenticated issue. For a new entered user the entry is like this: [...] So while authenticated is "0", the session entry is "1". This does not match. Maybe this is the reason why the values are lost?

Yes, this might be unexpected and existing values could get deleted on pivoting the session from unauthenticated to authenticated status. Still need to verify and find a better way.

12/21/11 01:43:28 changed by hasienda

The issue has been referred to in a comment to #6318 too.

02/03/12 00:59:50 changed by rjollos

  • cc set to rjollos.

02/23/12 02:35:39 changed by rjollos

#9755 closed as a duplicate.

(in reply to: ↑ 5 ) 02/23/12 18:32:22 changed by anonymous

Replying to hasienda:

Replying to stoecker:

NOTE: When I create a user as admin, authenticated is 0 as well.

Do you suggest, that we should lie about that again? In fact authenticated has been set before [10520], but starting with [10585] it's not authenticated, until the user has been really authenticated at least once.

Like in #9843, I am suggesting a clear "yes" (i.e. lie), and the most compelling reason is this: I want env.get_known_users() to list a user that just has been created by the admin, using this plugin's GUI. I can't see any reason why the user should be treated as "unknown" just after the admin created his or her account.

07/29/12 00:47:36 changed by peter@stuge.se

  • cc changed from rjollos to rjollos, peter@stuge.se.
  • summary changed from New user creation broken without verification turned on to All session attributes are deleted when user logs in first time.

07/29/12 00:48:57 changed by stuge

  • cc changed from rjollos, peter@stuge.se to rjollos, stuge.

07/29/12 00:51:51 changed by hasienda

I think, this is, what needs to be done then: 

--- accountmanagerplugin/trunk/acct_mgr/web_ui.py
+++ accountmanagerplugin/trunk/acct_mgr/web_ui.py
@@ -165,7 +165,7 @@
         cursor.execute("""
             INSERT INTO session
                     (sid,authenticated,last_visit)
-            VALUES  (%s,0,0)
+            VALUES  (%s,1,0)
             """, (username,))
 
     for attribute in ('name', 'email'):

07/29/12 03:57:32 changed by hasienda

(In [11826]) AccountManagerPlugin: Hotfix for pending user-registration issue, refs #9079, #9252, #9843 and #9090.

Now the reversion of [10520] done by [10585] is completed, although there are many related changes and improvements in-between, so this is not quite the same code as before.

Taking the chance to update now obsolete configuration examples in README too.

This has already open for much too long, so let's ditch the rather esoteric considerations of authenticated user entries in Trac db table session for now and move on.

Big sorry, that this took that much time, and thanks to Peter Stuge for finally pushing me to it tonight.

12/01/12 16:55:52 changed by hasienda

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

(In [12398]) AccountManagerPlugin: Releasing version 0.4, pushing development to acct_mgr-0.5dev.

Availability of that code as stable release closes #874, #3459, #4677, #5295, #5691, #6616, #7577, #8076, #8685, #8770, #8791, #8990, #9052, #9079, #9090, #9139, #9246, #9252, #9547, #9618, #9676, #9843, #9852, #9940, #10023, #10028, #10123, #10142, #10204, #10276, #10397, #10412, #10594, #10625 and #10644.

Some more issues have been worked-on, yet without confirmed resolution, refs #5464 (for JiraToTracIntegration), #8927 and #10134.

And finally there are some issues and enhancement requests showing progress, but known to require more work to resolve them satisfactorily, refs #843, #1600, #5964, #8217, #8933.

Thanks to all contributors and followers, that enabled and encouraged a good portion of this development work.

Add/Change #9252 (All session attributes are deleted when user logs in first time)




Change Properties
Action