Opened 13 years ago
Closed 12 years ago
#9252 closed defect (fixed)
All session attributes are deleted when user logs in first time
Reported by: | Dirk Stöcker | Owned by: | Steffen Hoffmann |
---|---|---|---|
Priority: | high | Component: | AccountManagerPlugin |
Severity: | normal | Keywords: | user creation |
Cc: | Ryan J Ollos, 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 (0)
Change History (19)
comment:1 follow-up: 5 Changed 13 years ago by
comment:2 follow-up: 4 Changed 13 years ago by
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.
comment:4 Changed 13 years ago by
Keywords: | user creation added |
---|
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
.
comment:5 follow-up: 14 Changed 13 years ago by
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.
comment:6 Changed 13 years ago by
Priority: | highest → 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?
comment:7 follow-up: 8 Changed 13 years ago by
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?
comment:8 Changed 13 years ago by
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 fromSessionStore
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.
comment:9 follow-up: 10 Changed 13 years ago by
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?
comment:10 Changed 13 years ago by
Status: | new → 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.
comment:12 Changed 13 years ago by
Cc: | Ryan J Ollos added; anonymous removed |
---|
comment:14 Changed 13 years ago by
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.
comment:15 Changed 12 years ago by
Cc: | stuge added |
---|---|
Summary: | New user creation broken without verification turned on → All session attributes are deleted when user logs in first time |
comment:16 Changed 12 years ago by
comment:17 Changed 12 years ago by
I think, this is, what needs to be done then:
-
accountmanagerplugin/trunk/acct_mgr/web_ui.py
165 165 cursor.execute(""" 166 166 INSERT INTO session 167 167 (sid,authenticated,last_visit) 168 VALUES (%s, 0,0)168 VALUES (%s,1,0) 169 169 """, (username,)) 170 170 171 171 for attribute in ('name', 'email'):
comment:18 Changed 12 years ago by
(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.
comment:19 Changed 12 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
(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.
NOTE: When I create a user as admin, authenticated is 0 as well.