Modify

Opened 8 years ago

Closed 4 years ago

Last modified 4 years ago

#131 closed enhancement (fixed)

[Patch] "Remember me"

Reported by: luks Owned by: pacopablo
Priority: highest Component: AccountManagerPlugin
Severity: normal Keywords:
Cc: gunnar, davidf@…, JamesMills, jasminlapalme@…, marcin, manski, hans@…, rjollos 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 luks 8 years ago.
remember-me-r3260.diff (2.7 KB) - added by jasminlapalme@… 6 years ago.
Patch for remember-me for r3260 and Trac 0.11
remember_me.patch (9.4 KB) - added by s0undt3ch 6 years ago.
Fixed a get_header bug introduced by me
remember_me.2.patch (10.3 KB) - added by s0undt3ch 6 years ago.
This one also checks against the ip stored on auth_cookie
remember_me.3.patch (10.5 KB) - added by dottedmag 6 years ago.
Updated patch against r3857
remember_me.4.patch (10.5 KB) - added by scratcher 6 years ago.
Patch to truly expire trac_auth_session cookie after logout
remember_me.5836.patch (8.6 KB) - added by manski 5 years ago.
Patch again r5836 and Trac 0.11.4; without IP checking

Download all attachments as: .zip

Change History (53)

comment:1 Changed 8 years ago by luks

  • Type changed from defect to enhancement

Changed 8 years ago by luks

comment:2 Changed 8 years ago by gunnar

  • Cc gunnar added
  • Trac Release set to 0.8

comment:3 Changed 7 years ago by otaku42

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 7 years ago by anonymous

0.10 version supported?

comment:5 Changed 7 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 7 years ago by trisweb

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

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

Replying to luks:

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

comment:8 Changed 7 years ago by anonymous

why not to include this patch into main plugin source?

comment:9 Changed 7 years ago by ThurnerRupert

#1428 marked as dup.

comment:10 Changed 6 years ago by davidf@…

  • Cc davidf@… added

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

  • Cc JamesMills added
  • Priority changed from normal to high

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

--JamesMills

Changed 6 years ago by jasminlapalme@…

Patch for remember-me for r3260 and Trac 0.11

comment:12 Changed 6 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 6 years ago by pacopablo

  • Owner changed from mgood to pacopablo
  • Trac Release changed from 0.8 to 0.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 6 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 6 years ago by 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 6 years ago by s0undt3ch

  • Trac Release changed from 0.10 to 0.11

Patch also makes this a 0.11 only feature.

Changed 6 years ago by s0undt3ch

Fixed a get_header bug introduced by me

comment:17 Changed 6 years ago by 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 6 years ago by s0undt3ch

This one also checks against the ip stored on auth_cookie

comment:18 Changed 6 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 6 years ago by marcin

  • Cc marcin 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 6 years ago by dottedmag

Updated patch against r3857

comment:20 Changed 6 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 follow-up: Changed 6 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 ; follow-up: Changed 6 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 6 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 6 years ago by scratcher

Patch to truly expire trac_auth_session cookie after logout

comment:24 Changed 5 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 5 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 5 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 5 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 5 years ago by sincoder

  • Priority changed from high to 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 5 years ago by manski

Patch again r5836 and Trac 0.11.4; without IP checking

comment:29 Changed 5 years ago by manski

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 5 years ago by manski

  • Cc manski added

comment:31 Changed 5 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 5 years ago by alexb

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

comment:33 Changed 5 years ago by hans@…

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

comment:34 Changed 5 years ago by anonymous

  • Cc hans@… added

comment:35 Changed 5 years ago by rjollos

  • Cc rjollos added

comment:36 Changed 4 years ago by sorin

  • Owner changed from pacopablo to sorin
  • Status changed from new to assigned

I would like this feature in main.

comment:37 Changed 4 years ago by anonymous

  • Summary changed from Patch: "Remember me" to [Patch] "Remember me"

comment:38 Changed 4 years ago by anonymous

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

comment:39 Changed 4 years ago by otaku42

  • Owner changed from sorin to pacopablo
  • Status changed from assigned to new

Assigning the ticket back to pacopablo.

comment:40 Changed 4 years ago by pacopablo

  • Resolution set to fixed
  • Status changed from new to closed

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

comment:41 Changed 4 years ago by anonymous

  • Resolution fixed deleted
  • Status changed from closed to 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 4 years ago by anatoly techtonik <techtonik@…>

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

comment:43 Changed 4 years ago by anonymous

  • Resolution set to fixed
  • Status changed from reopened to closed

comment:44 Changed 4 years ago by anonymous

This is working fine.

comment:45 Changed 4 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 4 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?

Add Comment

Modify Ticket

Action
as closed .
as The resolution will be set. Next status will be 'closed'.
to The owner will be changed from pacopablo. Next status will be 'closed'.
The resolution will be deleted. Next status will be 'reopened'.
Author


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

 
Note: See TracTickets for help on using tickets.