Modify

Opened 12 years ago

Closed 8 years ago

Last modified 7 years ago

#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)

remember-me.diff (3.0 KB) - added by Lukáš Lalinský 12 years ago.
remember-me-r3260.diff (2.7 KB) - added by jasminlapalme@… 10 years ago.
Patch for remember-me for r3260 and Trac 0.11
remember_me.patch (9.4 KB) - added by Pedro Algarvio, aka, s0undt3ch 9 years ago.
Fixed a get_header bug introduced by me
remember_me.2.patch (10.3 KB) - added by Pedro Algarvio, aka, s0undt3ch 9 years ago.
This one also checks against the ip stored on auth_cookie
remember_me.3.patch (10.5 KB) - added by Mikhail Gusarov 9 years ago.
Updated patch against r3857
remember_me.4.patch (10.5 KB) - added by scratcher 9 years ago.
Patch to truly expire trac_auth_session cookie after logout
remember_me.5836.patch (8.6 KB) - added by Sebastian Krysmanski 8 years ago.
Patch again r5836 and Trac 0.11.4; without IP checking

Download all attachments as: .zip

Change History (53)

comment:1 Changed 12 years ago by Lukáš Lalinský

Type: defectenhancement

Changed 12 years ago by Lukáš Lalinský

Attachment: remember-me.diff added

comment:2 Changed 11 years ago by Gunnar Wagenknecht

Cc: Gunnar Wagenknecht added; anonymous removed
Trac Release: 0.8

comment:3 Changed 11 years ago by Michael Renzmann

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 :)

comment:4 Changed 11 years ago by anonymous

0.10 version supported?

comment:5 Changed 11 years ago by trisweb

Any status on this? This needs to be added at least as an option to this plugin IMHO.

comment:6 Changed 11 years ago by trisweb

Patch works great on 0.10, fyi. I'm happy for now. :)

comment:7 in reply to:  description Changed 11 years ago by anonymous

Replying to luks:

Attached a patch to remember the login if user wants to.

comment:8 Changed 10 years ago by anonymous

why not to include this patch into main plugin source?

comment:9 Changed 10 years ago by rupert thurner

#1428 marked as dup.

comment:10 Changed 10 years ago by David Fraser

Cc: David Fraser added

comment:11 Changed 10 years ago by James.Mills@…

Cc: James Mills added
Priority: normalhigh

Does this patch work with the current trunk and Trac-0.11 ? We need this in the source!

--JamesMills

Changed 10 years ago by jasminlapalme@…

Attachment: remember-me-r3260.diff added

Patch for remember-me for r3260 and Trac 0.11

comment:12 Changed 10 years ago by anonymous

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 9 years ago by John Hampton

Owner: changed from Matt Good to John Hampton
Trac Release: 0.80.10

OK, I need some clarification on what exactly this is supposed to be doing.

Is it:

  1. 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?
  2. 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 9 years ago by james@…

Hi,

To clarify it should:

  1. Remember and Automatically log the user in when they re-visit the site.

--JamesMills

comment:15 Changed 9 years ago by Pedro Algarvio, aka, s0undt3ch

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 9 years ago by Pedro Algarvio, aka, s0undt3ch

Trac Release: 0.100.11

Patch also makes this a 0.11 only feature.

Changed 9 years ago by Pedro Algarvio, aka, s0undt3ch

Attachment: remember_me.patch added

Fixed a get_header bug introduced by me

comment:17 Changed 9 years ago by Pedro Algarvio, aka, s0undt3ch

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 9 years ago by Pedro Algarvio, aka, s0undt3ch

Attachment: remember_me.2.patch added

This one also checks against the ip stored on auth_cookie

comment:18 Changed 9 years ago by anonymous

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 9 years ago by Marcin Wojdyr

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).

Changed 9 years ago by Mikhail Gusarov

Attachment: remember_me.3.patch added

Updated patch against r3857

comment:20 Changed 9 years ago by scratcher

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 Changed 9 years ago by tim.kosse@…

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 in reply to:  21 ; Changed 9 years ago by scratcher

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 in reply to:  22 Changed 9 years ago by scratcher

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 9 years ago by scratcher

Attachment: remember_me.4.patch added

Patch to truly expire trac_auth_session cookie after logout

comment:24 Changed 9 years ago by sincoder

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 9 years ago by sincoder

I need to clarify that I get the above error when I visit my trac again on computer A

comment:26 Changed 9 years ago by jb@…

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 9 years ago by PeterA

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 9 years ago by sincoder

Priority: highhighest

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 8 years ago by Sebastian Krysmanski

Attachment: remember_me.5836.patch added

Patch again r5836 and Trac 0.11.4; without IP checking

comment:29 Changed 8 years ago by Sebastian Krysmanski

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 8 years ago by Sebastian Krysmanski

Cc: Sebastian Krysmanski added

comment:31 Changed 8 years ago by anonymous

Please make this part of the source!!! We would like the feature (it is common on almost all apps today).

comment:32 Changed 8 years ago by Alex Barrett

Any update on commiting this feature? It's been 15 months since "I'm happy to add this type of feature."

comment:33 Changed 8 years ago by hans@…

Please add this to the official plugin. This feature is much needed.

comment:34 Changed 8 years ago by anonymous

Cc: hans@… added

comment:35 Changed 8 years ago by Ryan J Ollos

Cc: Ryan J Ollos added

comment:36 Changed 8 years ago by Sorin Sbarnea

Owner: changed from John Hampton to Sorin Sbarnea
Status: newassigned

I would like this feature in main.

comment:37 Changed 8 years ago by anonymous

Summary: Patch: "Remember me"[Patch] "Remember me"

comment:38 Changed 8 years ago by anonymous

This is a very critical feature -- why is nothing happening?

comment:39 Changed 8 years ago by Michael Renzmann

Owner: changed from Sorin Sbarnea to John Hampton
Status: assignednew

Assigning the ticket back to pacopablo.

comment:40 Changed 8 years ago by John Hampton

Resolution: fixed
Status: newclosed

(In [7720]) Commit the Remember Me patch. Closes #131

comment:41 Changed 8 years ago by anonymous

Resolution: fixed
Status: closedreopened

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 8 years ago by anatoly techtonik

What version of Trac and Account Manager do you use? secure_cookies were added in 0.11.2

comment:43 Changed 8 years ago by anonymous

Resolution: fixed
Status: reopenedclosed

comment:44 Changed 8 years ago by anonymous

This is working fine.

comment:45 Changed 8 years ago by Ben

Is that 'patch' included in the newest version? I'm using TracAccountManager 0.2.1dev-r7163 and doesn't find this option...

comment:46 Changed 7 years ago by hans@…

I'm running Trac 0.11.7, with TracAccountManager 0.2.1dev-r7731 I don't see a "Remember me" checkbox on the login page. Do I need to change some settings to enable this?

Comment #40 says it was commited in r7720, so it should be included in the version I have right?

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain John Hampton.
The resolution will be deleted.

Add Comment


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

 
Note: See TracTickets for help on using tickets.