#131 closed enhancement (fixed)
[Patch] "Remember me"
Reported by: | Lukáš Lalinský | Owned by: | John Hampton |
---|---|---|---|
Priority: | highest | Component: | AccountManagerPlugin |
Severity: | normal | Keywords: | |
Cc: | Gunnar Wagenknecht, David Fraser, James Mills, jasminlapalme@…, Marcin Wojdyr, Sebastian Krysmanski, hans@…, Ryan J Ollos | Trac Release: | 0.11 |
Description
Attached a patch to remember the login if user wants to.
Attachments (7)
Change History (53)
comment:1 Changed 19 years ago by
Type: | defect → enhancement |
---|
Changed 19 years ago by
Attachment: | remember-me.diff added |
---|
comment:2 Changed 18 years ago by
Cc: | Gunnar Wagenknecht added; anonymous removed |
---|---|
Trac Release: | → 0.8 |
comment:3 Changed 18 years ago by
comment:5 Changed 18 years ago by
Any status on this? This needs to be added at least as an option to this plugin IMHO.
comment:7 Changed 18 years ago by
Replying to luks:
Attached a patch to remember the login if user wants to.
comment:10 Changed 17 years ago by
Cc: | David Fraser added |
---|
comment:11 Changed 17 years ago by
Cc: | James Mills added |
---|---|
Priority: | normal → high |
Does this patch work with the current trunk and Trac-0.11 ? We need this in the source!
Changed 17 years ago by
Attachment: | remember-me-r3260.diff added |
---|
Patch for remember-me for r3260 and Trac 0.11
comment:12 Changed 17 years ago by
Cc: | jasminlapalme@… added |
---|
I add the remember-me-r3260.diff attachment. This path works with the current trunk and trac-0.11.
Again why not adding this to the source.
comment:13 Changed 17 years ago by
Owner: | changed from Matt Good to John Hampton |
---|---|
Trac Release: | 0.8 → 0.10 |
OK, I need some clarification on what exactly this is supposed to be doing.
Is it:
- If the user closes the browser without logging out, the next time they open the browser and got to the site they will be automatically logged in?
- Remember the user's login name so that they only have to type in their password?
Also, I've got serious doubts about the effectiveness of the patches. First, they don't work for me. Second, the patches create another cookie, trac_auth_session
that appears to have the value of whether or not the session is "remembered", except it is a session cookie, which means it'll be deleted when the user closes the browser.
I'm happy to add this type of feature, but please clarify what the expected behavior is actually supposed to be.
comment:14 Changed 17 years ago by
Hi,
To clarify it should:
- Remember and Automatically log the user in when they re-visit the site.
comment:15 Changed 17 years ago by
This last patch, is updated to r3734, checks against current ip, logs user out if it doesn't match and makes this an optional feature.
comment:16 Changed 17 years ago by
Trac Release: | 0.10 → 0.11 |
---|
Patch also makes this a 0.11 only feature.
Changed 17 years ago by
Attachment: | remember_me.patch added |
---|
Fixed a get_header bug introduced by me
comment:17 Changed 17 years ago by
This can probably get simpler though, the info I store on the cookie is present on the auth_cookie
table, maybe we keep to a single cookie(plus the form token one) and check ip from db.
Changed 17 years ago by
Attachment: | remember_me.2.patch added |
---|
This one also checks against the ip stored on auth_cookie
comment:18 Changed 17 years ago by
Note that the new patch changes the meaning of the trac_auth_session
cookie. My original patch used it to check whether we are in a regular session, and if we are not to extend the expires header of the persistent cookie. This was to avoid sending a new cookie in each request.
The new patch makes trac_auth_session
a long-living cookie and uses it for a different reason, which I don't exactly understand, but I think it could be done without it (by comparing req.get_header("X-Forwarded-For") or req.remote_addr
to ipnr
from the auth_cookie
table). The new patch also updates the usual trac_auth
cookie on each request.
comment:19 Changed 16 years ago by
Cc: | Marcin Wojdyr added |
---|
The patch works great for people using a single IP, but a number of wxTrac users is complaining that it doesn't work. Those people either change networks or their ISP uses several IP addresses. They are logged off even without closing browser.
I'd really like to see this feature included in the plugin, but IMHO checking IP should be optional (or removed).
comment:20 Changed 16 years ago by
To dottedmag: Thanks for the patch! One problem - sometimes remember_me.3.patch causes internal server error which prevents user to log in until Trac cookies are removed. On my machine it's occured when I do log out:
2008-09-04 12:17:18,000 Trac[main] DEBUG: Dispatching <Request "GET u'/login'"> 2008-09-04 12:17:18,015 Trac[web_ui] DEBUG: Updating auth cookie cd7ed7b364780a791a7a01631a165bdd for user None 2008-09-04 12:17:18,015 Trac[chrome] DEBUG: Prepare chrome data for request 2008-09-04 12:17:18,015 Trac[web_ui] DEBUG: Updating auth cookie cd7ed7b364780a791a7a01631a165bdd for user None 2008-09-04 12:17:18,015 Trac[perm] DEBUG: No policy allowed anonymous performing TICKET_CREATE on None 2008-09-04 12:17:18,015 Trac[perm] DEBUG: No policy allowed anonymous performing TICKET_VIEW on None 2008-09-04 12:17:18,015 Trac[perm] DEBUG: No policy allowed anonymous performing BROWSER_VIEW on None 2008-09-04 12:17:18,015 Trac[perm] DEBUG: No policy allowed anonymous performing TRAC_ADMIN on None 2008-09-04 12:17:18,015 Trac[perm] DEBUG: No policy allowed anonymous performing PERMISSION_GRANT on None 2008-09-04 12:17:18,015 Trac[perm] DEBUG: No policy allowed anonymous performing PERMISSION_REVOKE on None 2008-09-04 12:17:18,015 Trac[perm] DEBUG: No policy allowed anonymous performing TICKET_ADMIN on None 2008-09-04 12:17:18,015 Trac[perm] DEBUG: No policy allowed anonymous performing TIMELINE_VIEW on None 2008-09-04 12:17:18,015 Trac[perm] DEBUG: No policy allowed anonymous performing ROADMAP_VIEW on None 2008-09-04 12:17:18,015 Trac[perm] DEBUG: No policy allowed anonymous performing WIKI_VIEW on <Resource 'wiki'> 2008-09-04 12:17:18,015 Trac[perm] DEBUG: No policy allowed anonymous performing SEARCH_VIEW on None 2008-09-04 12:17:18,015 Trac[perm] DEBUG: No policy allowed anonymous performing REPORT_VIEW on None 2008-09-04 12:17:18,015 Trac[perm] DEBUG: No policy allowed anonymous performing TESTMNG_ADMIN on None 2008-09-04 12:17:18,015 Trac[main] ERROR: 'NoneType' object is unsubscriptable Traceback (most recent call last): File "c:\python25\lib\site-packages\Trac-0.11-py2.5.egg\trac\web\main.py", line 423, in _dispatch_request dispatcher.dispatch(req) File "c:\python25\lib\site-packages\Trac-0.11-py2.5.egg\trac\web\main.py", line 173, in dispatch chosen_handler) File "c:\python25\lib\site-packages\Trac-0.11-py2.5.egg\trac\web\main.py", line 286, in _pre_process_request chosen_handler = filter_.pre_process_request(req, chosen_handler) File "build\bdist.win32\egg\acct_mgr\web_ui.py", line 461, in pre_process_request if not req.incookie['trac_auth_session'].value == row[0]: TypeError: 'NoneType' object is unsubscriptable
comment:21 follow-up: 22 Changed 16 years ago by
In LoginModule._do_logout, the session cookie needs to be reset even if the username is (still) set to anonymous. Else an auth cookie with an unmatching IP will never get reset leading to infinite redirect loop.
comment:22 follow-up: 23 Changed 16 years ago by
Replying to tim.kosse@filezilla-project.org:
In LoginModule._do_logout, the session cookie needs to be reset even if the username is (still) set to anonymous. Else an auth cookie with an unmatching IP will never get reset leading to infinite redirect loop.
Yep! I've just commented two lines in _do_logout and now it works for me, thanks!
#if req.authname == 'anonymous': # Not logged in #return
comment:23 Changed 16 years ago by
Replying to scratcher:
Yep! I've just commented two lines in _do_logout and now it works for me, thanks!
There was different reason for error obviously. My error message was caused by fact that trac_auth_session
cookie was not reseted at all in _do_logout
method even when anonymous check was removed (commented).
Aterwards I discovered that req.outcookie['trac_auth_session']['path']
is set in _do_logout
method along with 'expires'
property. When I removed first assignment ('path'
), trac_auth_session
cookie become removed correctly from browser and error gone. Now it works with Trac 0.11 stable.
Changed 16 years ago by
Attachment: | remember_me.4.patch added |
---|
Patch to truly expire trac_auth_session cookie after logout
comment:24 Changed 16 years ago by
I get this error if I've login with "remember me" on computer A and login in without "remember me" on computer B:
Oops…
Trac detected an internal error:
TypeError: unsubscriptable object
comment:25 Changed 16 years ago by
I need to clarify that I get the above error when I visit my trac again on computer A
comment:26 Changed 16 years ago by
I receive the same error as sincoder when svitching between pc's: I can solve it i Firefox by deleting the cookies. NOTE: using remember_me.4.patch
2008-11-05 10:30:43,947 Trac[main] ERROR: 'NoneType' object is unsubscriptable Traceback (most recent call last): File "/usr/lib/python2.5/site-packages/Trac-0.11.1-py2.5.egg/trac/web/main.py", line 423, in _dispatch_request dispatcher.dispatch(req) File "/usr/lib/python2.5/site-packages/Trac-0.11.1-py2.5.egg/trac/web/main.py", line 173, in dispatch chosen_handler) File "/usr/lib/python2.5/site-packages/Trac-0.11.1-py2.5.egg/trac/web/main.py", line 286, in _pre_process_request chosen_handler = filter_.pre_process_request(req, chosen_handler) File "build/bdist.linux-i686/egg/acct_mgr/web_ui.py", line 461, in pre_process_request if not req.incookie['trac_auth_session'].value == row[0]: TypeError: 'NoneType' object is unsubscriptable
comment:27 Changed 16 years ago by
I manually merged remember_me.4.patch to 0.11/acct_mgr and got it working great. Many thanks. Though each time my laptop got a new IP due to DNS, the session disappeared and I had to log in again.
Looks like the pre_process_request() does not honor the trac environment variable [trac].check_ip. If it is false, the function should skip checking for IP match. I commented out the fragment of the code below and got the issue resolved.
# IRequestFilter methods 440 def pre_process_request(self, req, handler): 441 ### if 'trac_auth_session' in req.incookie: 442 # Let's check for a matching IP, proxy'ed or not 443 ### remote_addr = req.get_header("X-Forwarded-For") or req.remote_addr 444 ### if not req.incookie['trac_auth_session'].value == remote_addr: 445 ### self.log.debug("Cookie IP does not match Remote address IP: " 446 ### "'%s'!='%s'; Killing Session", 447 ### req.incookie['trac_auth_session'].value, 448 ### remote_addr) 449 ### self._do_logout(req) 450 # Repeat request after logout in order for the user not to 451 # think he's logged in 452 ### req.redirect(req.path_info)
comment:28 Changed 16 years ago by
Priority: | high → highest |
---|
Please resolve the issue with switching PC's when using remember_me.4.patch.
2008-11-05 10:30:43,947 Trac[main] ERROR: 'NoneType' object is unsubscriptable Traceback (most recent call last): File "/usr/lib/python2.5/site-packages/Trac-0.11.1-py2.5.egg/trac/web/main.py", line 423, in _dispatch_request dispatcher.dispatch(req) File "/usr/lib/python2.5/site-packages/Trac-0.11.1-py2.5.egg/trac/web/main.py", line 173, in dispatch chosen_handler) File "/usr/lib/python2.5/site-packages/Trac-0.11.1-py2.5.egg/trac/web/main.py", line 286, in _pre_process_request chosen_handler = filter_.pre_process_request(req, chosen_handler) File "build/bdist.linux-i686/egg/acct_mgr/web_ui.py", line 461, in pre_process_request if not req.incookie['trac_auth_session'].value == row[0]: TypeError: 'NoneType' object is unsubscriptable
Changed 16 years ago by
Attachment: | remember_me.5836.patch added |
---|
Patch again r5836 and Trac 0.11.4; without IP checking
comment:29 Changed 16 years ago by
I've added a patch against the currently most recent AccountManagerPlugin revision (r5836) and Trac 0.11.4. This patch differs from the previous patch as it doesn't use IP address checking and has some documentation. I've disabled this "obstacle" as most/some(?) people have dynamic IP addresses and for those IP address checking renders this patch almost useless.
comment:30 Changed 16 years ago by
Cc: | Sebastian Krysmanski added |
---|
comment:31 Changed 15 years ago by
Please make this part of the source!!! We would like the feature (it is common on almost all apps today).
comment:32 Changed 15 years ago by
Any update on commiting this feature? It's been 15 months since "I'm happy to add this type of feature."
comment:33 Changed 15 years ago by
Please add this to the official plugin. This feature is much needed.
comment:34 Changed 15 years ago by
Cc: | hans@… added |
---|
comment:35 Changed 15 years ago by
Cc: | Ryan J Ollos added |
---|
comment:36 Changed 15 years ago by
Owner: | changed from John Hampton to Sorin Sbarnea |
---|---|
Status: | new → assigned |
I would like this feature in main.
comment:37 Changed 15 years ago by
Summary: | Patch: "Remember me" → [Patch] "Remember me" |
---|
comment:39 Changed 15 years ago by
Owner: | changed from Sorin Sbarnea to John Hampton |
---|---|
Status: | assigned → new |
Assigning the ticket back to pacopablo.
comment:40 Changed 15 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:41 Changed 15 years ago by
Resolution: | fixed |
---|---|
Status: | closed → reopened |
I get this error:
Traceback (most recent call last): File "/usr/lib/python2.4/site-packages/trac/web/api.py", line 339, in send_error 'text/html') File "/usr/lib/python2.4/site-packages/trac/web/chrome.py", line 684, in render_template data = self.populate_data(req, data) File "/usr/lib/python2.4/site-packages/trac/web/chrome.py", line 592, in populate_data d['chrome'].update(req.chrome) File "/usr/lib/python2.4/site-packages/trac/web/api.py", line 169, in __getattr__ value = self.callbacks[name](self) File "/usr/lib/python2.4/site-packages/trac/web/chrome.py", line 460, in prepare_request for category, name, text in contributor.get_navigation_items(req): File "/usr/lib/python2.4/site-packages/trac/web/auth.py", line 86, in get_navigation_items if req.authname and req.authname != 'anonymous': File "/usr/lib/python2.4/site-packages/trac/web/api.py", line 169, in __getattr__ value = self.callbacks[name](self) File "/usr/lib/python2.4/site-packages/trac/web/main.py", line 131, in authenticate authname = authenticator.authenticate(req) File "build/bdist.linux-x86_64/egg/acct_mgr/web_ui.py", line 381, in wrap File "build/bdist.linux-x86_64/egg/acct_mgr/web_ui.py", line 392, in authenticate File "/usr/lib/python2.4/site-packages/trac/web/auth.py", line 70, in authenticate authname = self._get_name_for_cookie(req, req.incookie['trac_auth']) File "build/bdist.linux-x86_64/egg/acct_mgr/web_ui.py", line 456, in _get_name_for_cookie AttributeError: 'Environment' object has no attribute 'secure_cookies'
comment:42 Changed 15 years ago by
What version of Trac and Account Manager do you use? secure_cookies
were added in 0.11.2
comment:43 Changed 15 years ago by
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
comment:45 Changed 15 years ago by
Is that 'patch' included in the newest version? I'm using TracAccountManager 0.2.1dev-r7163 and doesn't find this option...
Patch tested with current trunk, works fine here. From my point of view it would be a useful addition to the plugin, I hereby vote for it being included :)