Roundup Tracker - Issues

Issue 2550785

classification
Using login from search (or logout) fails.
Type: behavior Severity: normal
Components: Web interface Versions: 1.4
process
Status: fixed fixed
:
: rouilj : ber, ezio.melotti, rouilj
Priority: normal : patch

Created on 2012-12-01 02:42 by rouilj, last changed 2016-07-04 04:05 by rouilj.

Messages
msg4688 Author: [hidden] (rouilj) Date: 2012-12-01 02:42
Original posted to roundup-devel

If I am logged out of the roundup tracker and go to:

http://issues.roundup-tracker.org/issue?%40search_text=&title=&%40columns=title&id=&%40columns=id&creation=&creator=&activity=&%40columns=activity&%40sort=activity&actor=&nosy=&type=&components=&versions=&dependencies=&assignee=&keywords=1&priority=&%40group=priority&status=1&%40columns=status&resolution=&%40pagesize=50&%40startwith=0&%40action=search

(This url is the result of a search for issues with the patch
keyword.)

When I then try to login with that URL displayed in my browser I get:

    "broken form: multiple @action values submitted"

Can anybody reproduce this? If so I'll ticket it. I am using palemoon
12.3 (firefox derivative) on windows xp sp3.

If I chose the "open tickets" search link and try to log in I am
logged in just fine. 

This was confirmed so I am finally opening  ticket.

Further work/discussion at:
http://psf.upfronthosting.co.za/roundup/meta/issue448

The problem is that the login is a POST and includes an @action
variable. However the search result is a get style (so it can be
easily bookmarked) and the @action in the url is incorporated
in the url that the login post is sent to (as the action of the
form is the url "#").

So one possibility is to use as the login action the base url without
including any search term (after the ?).

In the 1.4.19 template it looks like this action is created by:

    tal:attributes="action request/base"

I would expect request/base would return the url before the ?
so it looks like this may be a bug in request/base.

Other thoughts welcome.
msg4689 Author: [hidden] (rouilj) Date: 2012-12-01 02:43
Forgot to mention that the roundup-devel email trail starts at:

http://old.nabble.com/Wierd-login-error-when-logging-in-from-a-custom-search-td34240977.html#a34241077
msg5147 Author: [hidden] (ezio.melotti) Date: 2014-09-29 12:44
I fixed this on bugs.python.org:
http://psf.upfronthosting.co.za/roundup/meta/issue448

There might be a better solution (e.g. avoid conflicts between GET and
POST in the first place), but what I applied is simple and does the job.
msg5694 Author: [hidden] (rouilj) Date: 2016-07-01 02:21
I think this will work. Replace the existing:

  <input type="hidden" name="__came_from" tal:attributes="value
string:${request/base}${request/env/PATH_INFO}">

with:

    <input type="hidden" name="__came_from"
      tal:condition="exists:request/env/QUERY_STRING"
      tal:attributes="value 
string:${request/base}${request/env/PATH_INFO}?${request/env/QUERY_STRING}">

    <input type="hidden" name="__came_from"
      tal:condition="not:exists:request/env/QUERY_STRING"
tal:attributes="value
      string:${request/base}${request/env/PATH_INFO}">

This will replicate the current page url with or without the the query
string, so it should redirect to the same page that you are currently
viewing.

Use a form with an action of /${request/env/PATH_INFO} as ezio
did, or ${request/base}${request/env/PATH_INFO}

I would like a solution for:

      tal:attributes="value
     
string:${request/base}${request/env/PATH_INFO}?${request/env/QUERY_STRING}">

to replace ${request/env/QUERY_STRING} with nothing if the
QUERY_STRING doesn't exist. In the current form it breaks with an
exception hence the need to check for the existence of
request/env/QUERY_STRING.

Can somebody test and see if this solves the issue?
msg5695 Author: [hidden] (rouilj) Date: 2016-07-01 02:40
Heh heh.

If you are at the URL:

   http://localhost:9017/demo/issue?@action=logout&...

and try to login you get logged out again as the __came_from
redirects to the logout action 8-).

I wonder if the login action should ignore a request to redirect
to @logout, or if I need to make the tal smarter to use
the base path if @logout is specified.

Suggestions?
msg5696 Author: [hidden] (rouilj) Date: 2016-07-01 03:07
Well here is some smarter tal.

    <tal:if tal:condition="exists:request/env/QUERY_STRING">
       <input
tal:condition="python:request.env['QUERY_STRING'].find('@action=logout')
== -1"
	      type="hidden" name="__came_from"
	      tal:attributes="value
	     
string:${request/base}${request/env/PATH_INFO}?${request/env/QUERY_STRING}">
       <input
tal:condition="python:request.env['QUERY_STRING'].find('@action=logout')
!= -1"
	      type="hidden" name="__came_from"
	      tal:attributes="value
	      string:${request/base}${request/env/PATH_INFO}">
    </tal:if>
    <input type="hidden" name="__came_from"
	   tal:condition="not:exists:request/env/QUERY_STRING"
	   tal:attributes="value string:${request/base}${request/env/PATH_INFO}">


it looses the page you are on when you login after logging out,
but I figure it serves you right for logging out of roundup 8-).

Anybody want to try this code?
msg5702 Author: [hidden] (rouilj) Date: 2016-07-02 01:42
Hi all:

I think I have improved this situation.

At this point any login attempt will keep the user to the page where
they filled in the login form. If the login is invalid, it will pop up
a message reporting an invalid login and keep you on the same page,

If you log out and then login again, you will end up on the page you
were on before you logged out. Logout sends you to an index page (so
you aren't accidentally left on a page you think you can edit), but it
will keep the __came_from value for the page before the logout.

To make this work I have template patches and some code changes for
the LoginAction. The templates need to be patched with:

  <input type="hidden" name="__came_from"
    tal:condition="exists:request/env/QUERY_STRING"
    tal:attributes="value
string:${request/base}${request/env/PATH_INFO}?${request/env/QUERY_STRING}">
  <input type="hidden" name="__came_from"
    tal:condition="not:exists:request/env/QUERY_STRING"
   tal:attributes="value string:${request/base}${request/env/PATH_INFO}">

which replaces the current tal that sets __came_from. Also the login
form action needs to be request/base or some other safe endpoint that
will not send query arguments (so # is right out).

In addition you need my patches to cgi/actions.py to:

  * make login after a logout work (the code patch removes
      @action=logout from __came_from rather than the ugly tal I posted
      prior in this issue)
  * redirection on login failure

I am going to do some more manual testing to augment the 6 or so
additions to the test suite then check my changes in.

Does anybody want to review/test my changes?

-- rouilj
msg5734 Author: [hidden] (rouilj) Date: 2016-07-04 04:05
I am claiming this is mostly fixed.

Checkin: 894aa07be6cb
History
Date User Action Args
2016-07-04 04:05:01rouiljsetstatus: open -> fixed
resolution: fixed
messages: + msg5734
2016-07-02 01:42:55rouiljsetmessages: + msg5702
2016-07-01 03:07:38rouiljsetmessages: + msg5696
2016-07-01 02:40:28rouiljsetmessages: + msg5695
2016-07-01 02:25:48rouiljsetkeywords: + patch
assignee: rouilj
status: new -> open
2016-07-01 02:21:57rouiljsetmessages: + msg5694
2014-09-29 12:44:46ezio.melottisetnosy: + ezio.melotti
messages: + msg5147
2012-12-03 10:26:36bersetnosy: + ber
2012-12-01 02:43:19rouiljsetmessages: + msg4689
2012-12-01 02:42:09rouiljcreate