Warning: Can't synchronize with repository "(default)" (Unsupported version control system "svn": No module named svn). Look in the Trac log for more information.

Ticket #2114 (closed defect: fixed)

Opened 11 years ago

Last modified 10 years ago

Fix logout_handler to respect script path

Reported by: mramm Owned by: Gustavo
Priority: high Milestone: 2.0b5
Component: TurboGears Version: trunk
Severity: normal Keywords: upstream, repoze.what.plugins.sql, repoze.who
Cc:

Description

In #2104 there was a report that logout_handler is not appending the SCRIPT_PATH to the url that the user is redirected to, which breaks apps not mounted at the root of the path.

Attachments

repozewho-tg-2114.diff Download (15.7 KB) - added by Gustavo 10 years ago.
Patch to repoze.who v1.0.10

Change History

comment:1 Changed 11 years ago by mramm

  • Owner changed from faide to Gustavo

comment:2 Changed 11 years ago by Gustavo

  • Status changed from new to assigned

Hopefully tomorrow I'll have little yet enough time to solve this. In the mean time, one solution is to define the form plugin by ourselves and pass it to r.who through yourapp.config.app_cfg:

base_config.sa_auth.form_plugin = your_configured_RedirectingFormPlugin

comment:3 follow-up: ↓ 4 Changed 11 years ago by lszyba1

I think this bug is somewhere here:  http://repoze.org/viewcvs/repoze.who/trunk/repoze/who/plugins/form.py?rev=2882&view=markup

near:

if path_info == self.logout_handler_path:

but not sure 100%.

Let me know when you would be able to fix this. That is the only thing left that is stopping me from switching from tg1 to tg2.

Thanks, Lucas

comment:4 in reply to: ↑ 3 Changed 11 years ago by Gustavo

  • Keywords upstream, repoze.what.plugins.sql added
  • Status changed from assigned to closed
  • Resolution set to fixed

This has been solved in repoze.what.plugins.sql-1.0a4.

To use it on TG2, you may use the following settings (which are all optional) in your_app.config.app_cfg:

base_config.sa_auth.login_url = '/myapp/login'
base_config.sa_auth.login_handler = '/myapp/login_handler'
base_config.sa_auth.logout_url = '/myapp/logout'

I'm adding this info into the TG2 docs right now.

Replying to lszyba1:

I think this bug is somewhere here:

There was no bug in repoze.who, it was just that the repoze.what SQL plugin did not have the ability to set custom login/logout related URLs.

Enjoy!

comment:5 Changed 11 years ago by Gustavo

  • Status changed from closed to reopened
  • Resolution fixed deleted

Hmmm, I'm reopening this to make TG use this new feature in the repoze.what SQL plugin to insert the script path automatically.

comment:6 Changed 11 years ago by mramm

  • Priority changed from normal to high

comment:7 follow-up: ↓ 9 Changed 11 years ago by lszyba1

So the current state of the bug is that its fixed in repoze.what sql plugin and we are waiting for a change in tg2 to pass the script name?

Thanks, Lucas

comment:8 Changed 11 years ago by lszyba1

any news on this? thanks

comment:9 in reply to: ↑ 7 Changed 11 years ago by Gustavo

Replying to lszyba1:

So the current state of the bug is that its fixed in repoze.what sql plugin and we are waiting for a change in tg2 to pass the script name?

Right, and I'm stuck because of this:  http://groups.google.com/group/turbogears-trunk/browse_thread/thread/617e81dab2007180#

comment:10 Changed 11 years ago by mramm

  • Milestone changed from 2.0b2 to 2.0b3

comment:11 follow-up: ↓ 12 Changed 10 years ago by lszyba1

In the patch from the thread above "tg-patch-2114.diff" you seem to depend on the script name to be combined on tg side before it's passed into repoze.what.sql as final one url. Can we change the patch so that we somehow pass the script name to repoze.what, then url of the login/logut gets combined on repoze.what side.

Thanks, Lucas

comment:12 in reply to: ↑ 11 Changed 10 years ago by Gustavo

Hello, Lucas.

Replying to lszyba1:

In the patch from the thread above "tg-patch-2114.diff" you seem to depend on the script name to be combined on tg side before it's passed into repoze.what.sql as final one url.

Right.

Can we change the patch so that we somehow pass the script name to repoze.what, then url of the login/logut gets combined on repoze.what side.

repoze.what has nothing to do with that -- it only deals with authorization.

That patch seems like the best way to go to me, but unfortunately nobody seem to know why tg.url doesn't work there (or at least nobody has explained so). If I don't get help to get tg.url to work there, then my Option B is to modify repoze.who.plugins.form.RedirectingFormPlugin? to make it accept an optional callable that returns the base URL for the current application which would be used to determine the login/out URLs -- I can provide the patch but I'm not sure if Chris McDonough? is going to apply it (I don't find this very elegant, to be honest).

I'd rather deal with this in TG itself, but I need help.

Cheers!

comment:13 Changed 10 years ago by lszyba1

I've tried doing: from tg.controllers import url

and it works. Can you provide the full log, of the whole installation process. From start to finish. Thanks, Lucas

comment:14 Changed 10 years ago by Gustavo

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

I've just sent a patch to fix this in repoze.who, so this should get solved in the next repoze.who release.

comment:15 Changed 10 years ago by Gustavo

  • Keywords repoze.what.plugins.sql, repoze.who added; repoze.what.plugins.sql removed

comment:16 follow-up: ↓ 17 Changed 10 years ago by lszyba1

is tg or tg trunk sending the proper scriptname? where did you put that in? Can you send me a link to a tg svn?

Should I test it with current tg/repoze?

Thanks, Lucas

comment:17 in reply to: ↑ 16 Changed 10 years ago by Gustavo

Replying to lszyba1:

is tg or tg trunk sending the proper scriptname? where did you put that in? Can you send me a link to a tg svn?

Should I test it with current tg/repoze?

As I mentioned in the message above, the bug is on repoze.who, there's nothing to do from our side.

Unfortunately, due to time constraints, Chris (the repoze.who developer) has been unable to review and apply the patch I sent to him. I talked with him and he said he'll take care of it soon, though.

For the time being, I'm going to upload the patch so that you apply it against your repoze.who copy.

Changed 10 years ago by Gustavo

Patch to repoze.who v1.0.10

comment:18 Changed 10 years ago by lszyba1

Can we replace scritp_path with script_name. From repoze.org mailing list....Since the repoze.who quickstart plugin overwrites the repoze.who, they said that it is sufficient to provide the script_name capabilities in quickstart.

Gustavo, Can we get a new release version of the quickstart plugin.

Thanks, Lucas

Note: See TracTickets for help on using tickets.