#2210 closed defect (fixed)
After redirecting to the login screen, trac always loads the default page.
Reported by: | Owned by: | ejucovy | |
---|---|---|---|
Priority: | normal | Component: | PermRedirectPlugin |
Severity: | normal | Keywords: | patch |
Cc: | Sascha Silbe, ejucovy, Ryan J Ollos, Steffen Hoffmann | Trac Release: | 0.10 |
Description
If you follow a link (from an email for example) the login page comes up as expected, but after logging in the specific link is forgotten and you are taken to the default page (main wiki page).
I'm not sure if this is specifically a PermRedirectPlugin problem or a Trac problem in general, but it does affect RSS feeds quite a bit.
Attachments (3)
Change History (43)
comment:1 Changed 17 years ago by
comment:2 Changed 17 years ago by
I've attached a patch for a fix. When you request a page and fail authentication, it sets a cookie to the URL you asked for. A wrapper around LoginModule._redirect_back adjusts the Referer
header if it finds that cookie. _redirect_back
then redirects back to the original page if authentication succeeds.
comment:3 Changed 16 years ago by
I couldn't get your patch file to apply to v0.11 of PermRedirectPlugin - I think the line numbering might be out?
comment:4 follow-up: 5 Changed 16 years ago by
Hmm... no, I think your patch must be for an earlier version.
comment:5 Changed 16 years ago by
Replying to anonymous:
Hmm... no, I think your patch must be for an earlier version.
It's a patch for 0.10 - you'll have to write your own patch for 0.11
comment:6 Changed 16 years ago by
This could now be fixed by using the referer=
argument to /login
, added to Trac 0.11-stable in [T7493].
comment:7 Changed 16 years ago by
Hi, Sir,
If you can provide a patch for 0.11 independently? We don't want to update the Trac, only want the the redirect problem fix. I searched but can't find the patch down. THanks in advance.
Bill
Changed 16 years ago by
Attachment: | PermRedirect-urlfix.diff added |
---|
Patch to the 0.11 branch to use the referer querystring parameter
comment:8 Changed 16 years ago by
The patch I added above implements what rblank referred to in comment:6
comment:9 Changed 16 years ago by
After implement the PermRedirect-urlfix.diff , I got this. I am not developer, could you check what it is please?
Oops... Trac detected an internal error: invalid syntax (filter.py, line 27) Traceback (most recent call last): File "/usr/lib/python2.4/site-packages/trac/admin/templates/deploy_trac.cgi", line 27, in ? cgi_frontend.run() File "/usr/lib/python2.4/site-packages/trac/web/cgi_frontend.py", line 71, in run gateway.run(dispatch_request) File "/usr/lib/python2.4/site-packages/trac/web/wsgi.py", line 87, in run response = application(self.environ, self._start_response) File "/usr/lib/python2.4/site-packages/trac/web/main.py", line 381, in dispatch_request env = open_environment(env_path, use_cache=not run_once) File "/usr/lib/python2.4/site-packages/trac/env.py", line 571, in open_environment env = Environment(env_path) File "/usr/lib/python2.4/site-packages/trac/env.py", line 185, in __init__ load_components(self, plugins_dir and (plugins_dir,)) File "/usr/lib/python2.4/site-packages/trac/loader.py", line 108, in load_components loadfunc(env, search_path, auto_enable=plugins_dir) File "/usr/lib/python2.4/site-packages/trac/loader.py", line 65, in _load_eggs entry.load(require=True) File "/usr/lib/python2.4/site-packages/pkg_resources.py", line 1912, in load entry = __import__(self.module_name, globals(),globals(), ['__name__']) File "/usr/lib/python2.4/site-packages/permredirect/filter.py", line 27 login_url = req.href.login(referer=req.base_url + req.path_info) ^ SyntaxError: invalid syntax
comment:10 Changed 16 years ago by
I am using AccountManager, so can't load trac.web.auth LoginModule in your first patch. If this will have problem for your new patch?
Bill
comment:11 Changed 16 years ago by
Replying to Bill :
After implement the PermRedirect-urlfix.diff , I got this. I am not developer, could you check what it is please?
File "/usr/lib/python2.4/site-packages/permredirect/filter.py", line 27 login_url = req.href.login(referer=req.base_url + req.path_info) ^ SyntaxError: invalid syntax
Not too sure here; the best I can suggest is to check the syntax in the resulting file, maybe the patch applied incorrectly? I inserted that line 27 ('login_url...') in the patch, make sure it is indented the same as the line above('exctype...') and below it('if...').
Replying to anonymous:
I am using AccountManager, so can't load trac.web.auth LoginModule in your first patch. If this will have problem for your new patch?
Bill
The two patches proposed here are entirely independent. If you have already applied attachment:permredirect.diff you should get a clean version(I have this one) of PermRedirectPlugin before applying attachment:PermRedirect-urlfix.diff. I don't reference LoginModule anywhere in this code; other dependencies/incompatibilities I don't know anything about.
comment:12 Changed 16 years ago by
I have rechecked the /usr/lib/python2.4/site-packages/permredirect/filter.py and sure it is same with your PermRedirect-urlfix.diff.
Before when I search for this problem, I did see one patch for AccountManager plugin. But that patch was old and didn't work ( I tested by myself).
PermRedirect plugin should fix this problem, that is why I installed it, but still have some problem. If anyone in your group and help to figure this out? Thanks anyway.
Bill
comment:13 Changed 16 years ago by
I can confirm that the PermRedirect-urlfix.diff patch works like a charm for 0.11. What is needed to have this applied to the repository? Can I assist in any way?
Changed 16 years ago by
Attachment: | PermRedirect-urlfix.2.diff added |
---|
Patch to the 0.11 branch to use the referer querystring parameter (now slightly improved to also capture the querystring of the original request instead of just the request path).
comment:14 follow-up: 16 Changed 16 years ago by
Is there any change for this patch to be included in Trac trunk ? If not, why ?
I am using Trac trunk (0.12dev) and have a similar problem : the redirect is not necessarily to the start page but if you were in an "edit" page (wiki edit, ticket modification) and when submitting your changes a perm redirect occurs, the modification is lost and you have to reapply it.
If you think this is not the same problem, I'll open a new ticket.
Cheers,
Michel
comment:15 Changed 16 years ago by
The situation you describe doesn't make any sense though, since there is no permission check that could fail at that time that wouldn't fail when opening the edit screen (unless you manually delete the auth cookie I suppose).
comment:16 Changed 16 years ago by
Replying to jouvin@lal.in2p3.fr:
Is there any change for this patch to be included in Trac trunk ? If not, why ?
I made the patch against browser:permredirectplugin/0.11#4929 running against trac version 0.11stable-r7510 (from about). I was just able to update my permredirectplugin checkout to browser:permredirectplugin/0.11#5162 (doesn't appear to be any changes). We've been using this for the past couple of months and are very happy with it. coderanger: is there anything blocking acceptance of this patch?
I am using Trac trunk (0.12dev) and have a similar problem : the redirect is not necessarily to the start page but if you were in an "edit" page (wiki edit, ticket modification) and when submitting your changes a perm redirect occurs, the modification is lost and you have to reapply it.
If you think this is not the same problem, I'll open a new ticket.
The fact that your permission has expired is definitely a different problem. I'm somewhat in agreement with coderanger it doesn't make much sense; the only thing I could think of would be if your permission token expired due to time. Hitting refresh before editing might help? I've let tickets sit for a while without doing anything and then edited without a problem but this might be very dependent on what authentication scheme you are using.
Because ticket modification data is POSTed whereas this is just doing redirects (GET) that don't carry the post data along with it, this plugin (with or without the proposed patch here) will *not* solve the lost data problem.
Event Order
- Trac has a permission error
- PermRedirectPlugin without the above patch, no referrer info is passed (in our setup at least). This is confirmed through firebug and apache logs.
- With the patch, when we do the redirect we add a QueryString parameter named "referer" with the value of the page that was requested.
- Trac w/o the plugin renders an error page, that when you click the link to go to login, the Http Header "referer" is added because you followed a standard link.
- Login url does what it is supposed to (http basic auth over ssl in my setup).
- When you get to the login page under recent Tracs(slightly after .11 was released there was a fixup patch so that it would read referer from all params not just the headers) it redirects you to whatever location is given in the "referer" request parameter.
In any of the above scenarios under point 1, POST data is lost. From what I know of HTTP redirects there isn't a good way to thread the POST data. A couple not good options: keeping it in the server's session memory, sticking it in a cookie, trying to create querystring parameters out of everything posted.
comment:17 Changed 16 years ago by
Cc: | Sascha Silbe added; anonymous removed |
---|
comment:18 Changed 16 years ago by
I applied UnwashedMeme's PermRedirect-urlfix.2.diff patch, works great against Trac 0.11.2.
comment:19 Changed 15 years ago by
Keywords: | patch added |
---|
Is there any chance of getting this patch applied?
We have been using it successfully for months, it sounds like others have had success as well, and it just worked again on an entirely fresh install.
Thanks,
Russ
comment:20 Changed 14 years ago by
How is this plugin and this patch supposed to work? (It seem it doesn't work with me)
With this plugin the url of the login page has become something like this:
https://my.trac.example.com/login?referer=http%253A%252F%252Fmy.trac.example.com%252Froadmap
But the inner workings of the /login page seems not to react on any of this ?referer=
parameter.
Has anybody been getting this to work?
Thanks,
Dirk
comment:21 Changed 14 years ago by
Dirk,
It's been working fine for us for quite a while-- really wish we could get this patch applied to the plugin.
You need to apply the patch PermRedirect-urlfix.2.diff to the plugin before installing it with python easy_install-- looks like you might already have that point. The login url you gave looks correct; that's mostly what this plugin+patch provides.
Beyond that we rely on handling introduced in trac in [T7493] (comment:6)-- that was sometime during early trac 0.11. The login page at this point is supposed to, on successful login, redirect you back to the page specified by the referer.
Are you using a recent-ish trac (0.11+) with this plugin and patch applied? What happens after you login do you land on default page, are you still on the login page? Do you have any other plugins that are trying to intercept/munge the login procedure?
comment:22 follow-up: 23 Changed 14 years ago by
I'm running Trac 11.4. And I have the Account Manager plugin enabled. Could this be interfering with the login in an unwanted way?
Thanks, Dirk
comment:23 Changed 14 years ago by
Replying to labs@ixopusada.com:
I'm running Trac 11.4. And I have the Account Manager plugin enabled. Could this be interfering with the login in an unwanted way?
Yeah, I've heard reports of that plugin interfering especially if you have it configured for forms auth. I don't see a reason why the forms auth shouldn't be able to work with this, just reports that it doesn't. That will be a bug report over there, that it is ignoring the referer variable added in [T7493].
comment:24 Changed 14 years ago by
Getting back at this problem. I posted the presumed bug at the AccountManager ticket about this #3783, but nobody there responded. Now getting fed up with this and so I'm looking to solve it myself. :-)
After some digging, it seems that the ?referer=
parameter is being received by the plugin. It's just tripping over the url quotes.
Also the code in [T7493] doesn't seem to unquote the referer parameter.
Is that correct? Where is the unquoting suppose to happen? The referer is now http%3A%2F%2Fmy.trac.example.com%2Froadmap
(as seen inside the _redirect_back method)
Thanks,
Dirk
comment:25 Changed 14 years ago by
Confirming the upgrade to trac 0.12 went fine. Plugin with patch attachment:PermRedirect-urlfix.2.diff still works great for us.
any chance of getting it applied coderanger?
comment:26 Changed 14 years ago by
Ok. I finally fixed my issues (involving http <-> https and the AccountManagerPlugin).
The how to and the patch can be found here: #3783
comment:27 Changed 13 years ago by
AccountManagerPlugin accepts the referer argument now, but PermRedirectPlugin still doesn't generate the argument.
Any chance this will get fixed?
comment:28 Changed 12 years ago by
Cc: | ejucovy added |
---|
comment:29 Changed 12 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
(In [12073]) pass along referer (including query string) when redirecting to the login screen, so that after login trac will send the user back to the page he was trying to visit. fixes #2210. (thanks to nathan@acceleration.net and UnwashedMeme for the patch)
comment:30 Changed 12 years ago by
I've just adopted this plugin. I've implemented UnwashedMeme's patch on the newly created trunk of this plugin's source, and I'll tag and release a new version of the plugin ASAP.
I've also created a new ticket #10393 to track the separate issue of POST data loss described by Michel and UnwashedMeme in comment:14 and comment:16 -- I think it's fair to call it a bug so I'll follow up on that new ticket.
comment:31 follow-up: 32 Changed 12 years ago by
Cc: | Ryan J Ollos Steffen Hoffmann added |
---|---|
Resolution: | fixed |
Status: | closed → reopened |
comment:32 Changed 12 years ago by
Owner: | changed from Noah Kantrowitz to ejucovy |
---|---|
Status: | reopened → new |
Replying to sdegrande:
It does still not work for me, and I had to add a call to
urllib.unquote()
inauth.LoginModule._referer()
.Or am I missing something ?
I'm using Trac 1.0, with PermRedirect r12073 and AccountManager r12139.
Hmm ... I haven't been able to reproduce the problem with an environment like you describe.
Can you describe a little bit more about your situation? When you added a call to urllib.unquote()
(I'm assuming you mean in the trac core source code) did that fix the problem? Can you post a diff to show exactly where you added the unquote()
call?
Could you also insert a pdb.set_trace()
in acct_mgr/web_ui.py
at L560 (just after the referer = self._referer(req)
block) and see what the value of the referer
variable is there when your unquote()
patch is applied as well as when it is not? It would also be helpful to know the value of str(req.abs_href())
at that point as well.
comment:33 follow-up: 34 Changed 12 years ago by
I will do ASAP, but here are some more informations.
For that project, there are only authenticated accesses (so there are no permissions for 'anonymous').
The recipe is:
- the user is logged in
- the user is displaying the timeline page, for example
- the user clicks on the 'logout' button
- the 'login page' of AccountManager is displayed (due to PermRedirect)
With old installation (Trac 0.11), the URL displayed by the browser is
https://the_host/the_project/login
and the HTML form contains
<input type="hidden" name="referer" value="http://the_host/the_project/timeline" />
With new installation (Trac 1.0), the URL displayed by the browser is
https://the_host/the_project/login?referer=http%253A%252F%252Fthe_host%252Fthe_project%252Ftimeline
and the HTML form contains [[BR]
<input name="referer" value="http%3A%2F%2Fthe_host%2Fthe_project%2Ftimeline" type="hidden">
Since the 'referer' param seems to be double-encoded, I manually removed the double-encoding from the URL and pointed my browser to
https://the_host/the_project/login?referer=http%3A%2F%2Fthe_host%2Fthe_project%252Ftimeline
and now the HTML login form contains
<input type="hidden" name="referer" value="http://the_host/the_project/timeline" />
and the redirection works as expected.
So it seems that the issue is due to the double-encoding of the URL params. I can try to find where in the code that double-encoding is created, if you need.
comment:34 follow-up: 35 Changed 12 years ago by
Replying to sdegrande:
Thanks, this is very helpful.
I see you're using HTTPS for login and HTTP for the rest of the site. I suspect that's relevant to the issue. http://trac-hacks.org/ticket/3783#comment:6 describes the same problem with the same setup, and although that ticket is closed, I can't find any existing analysis of where the double-quoting was occurring.
So it seems that the issue is due to the double-encoding of the URL params. I can try to find where in the code that double-encoding is created, if you need.
That would be very helpful, thanks.
If I have time over the next few days I'll try to configure HTTPS-on-login for my own Trac installs and see if I can reproduce the problem under that setup. How did you configure the "HTTPS on /login" behavior? Are you using an Apache rewrite rule on /login or some other method?
comment:35 Changed 12 years ago by
Replying to ejucovy:
I see you're using HTTPS for login and HTTP for the rest of the site. I suspect that's relevant to the issue. http://trac-hacks.org/ticket/3783#comment:6 describes the same problem with the same setup, and although that ticket is closed, I can't find any existing analysis of where the double-quoting was occurring.
Interesting ! I will try without the https rewriting.
Are you using an Apache rewrite rule on /login or some other method?
That's Apache rewriting yes :
# Redirect all login pages to https RewriteCond %{HTTPS} off RewriteRule ^/([^/]+)/login$ https://%{HTTP_HOST}:443/$1/login [last]
(HTTPS being rewrote back to HTTP for the other pages)
comment:36 Changed 12 years ago by
Status: | new → assigned |
---|
On my own trac environment I just tried setting up HTTPS for login using the Apache rewrite rule you provided, and I'm now seeing the error too! So it's definitely related to this somehow.
The solution there says to add the NE flag to the rewrite rule, so it should read:
RewriteRule ^/([^/]+)/login$ https://%{HTTP_HOST}:443/$1/login [L,NE]
I just tried this, and it fixed the double-encoding problem for me.
I'll leave this open until you confirm that it fixes the problem for you as well.
comment:37 follow-up: 39 Changed 12 years ago by
It definitively fixes the issue.
I would never have thought that mod_rewrite was the source of the problem...
So, in my opinion, you can close the ticket.
Thanks a lot.
comment:38 Changed 12 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
comment:39 follow-up: 40 Changed 12 years ago by
Replying to sdegrande:
It definitively fixes the issue.
I would never have thought that mod_rewrite was the source of the problem...
Congrats for finally resolving this issue.
Would you mind adding a hint on this finding to the wiki page then? So other Apache users attempting a similar setup wouldn't fall into the same trap, or at least have a second chance to resolve their problem, apart from this ancient ticket. Would have done this on my own, but I don't use Apache, so my wording would probably be not as clear as required. Thanks in advance for consideration.
comment:40 Changed 12 years ago by
Replying to hasienda:
Would you mind adding a hint on this finding to the wiki page then? So other Apache users attempting a similar setup wouldn't fall into the same trap, or at least have a second chance to resolve their problem, apart from this ancient ticket. Would have done this on my own, but I don't use Apache, so my wording would probably be not as clear as required. Thanks in advance for consideration.
Thanks -- good idea. I just added a hint about this in wiki:PermRedirectPlugin#FrequentlyAskedQuestions (well, it's the only frequently asked question so far :-) and in wiki:PermRedirectPlugin#HTTPSOnly -- hopefully using enough keywords to make the solution appear in Google results.
I also posted about the problem and solution on the related t.e.o ticket: http://trac.edgewall.org/ticket/4733#comment:23
I also just implemented a Trac-only solution to the "HTTPS only" use-case. This is a new feature in PermRedirectPlugin. It's described at wiki:PermRedirectPlugin#HTTPSOnly and #10642, with additional details of the motivation in http://trac.edgewall.org/ticket/4733#comment:24.
For anyone who uses PermRedirectPlugin + AccountManager together, I think this is a better and easier approach. With this new feature installed, only reason to use Apache RewriteRules
instead would be in situations where login is handled at the Apache level instead of AccountManager's login component.
This is due to how the HTTP referer header works with redirects. Unfortunately there isn't much I can do.