PR#121: Retain transaction ID through 401 pages - ipsilon

ipsilon

#121 Retain transaction ID through 401 pages

Merged 7 years ago by merlinthp. Opened 7 years ago by merlinthp.

merlinthp/ipsilon tid into master

Retain transaction ID through 401 pages

Howard Johnson • 7 years ago

4d1f9f0

ipsilon/util/errors.py

file modified

+7 -1

		`@@ -1,6 +1,7 @@`
		`# Copyright (C) 2014 Ipsilon project Contributors, for license see COPYING`

		`from ipsilon.util.page import Page`
		`+ import cherrypy`


		`class Errors(Page):`
		`@@ -34,8 +35,13 @@`
		`class Error_401(Errors):`

		`def handler(self, status, message, traceback, version):`
		`+ try:`
		`+ tid = self.get_valid_transaction('login').transaction_id`
		`+ except cherrypy.HTTPError:`
		`+ tid = None`
		`return self._error_template('unauthorized.html',`
		`- title='Unauthorized', message=message)`
		`+ title='Unauthorized', message=message,`
		`+ ipsilon_transaction_id=tid)`


		`class Error_404(Errors):`

templates/unauthorized.html

file modified

		`@@ -7,6 +7,10 @@`
		`{% else %}`
		`<p>Authentication was not successful</p>`
		`{% endif %}`
		`+ {% if ipsilon_transaction_id %}`
		`+ <p><a href="{{ basepath }}/login?ipsilon_transaction_id={{ ipsilon_transaction_id }}" title="Login">Try to login again</a></p>`
		`+ {% else %}`
		`<p><a href="{{ basepath }}/login" title="Login">Try to login again</a></p>`
		`+ {% endif %}`
		`</div>`
		`{% endblock %}`

merlinthp commented 7 years ago

If we get a 401 during a transaction (i.e. failing authentication),
ipsilon shows the 401 error page, which has a link back to the login
page. Clicking this link causes a new transaction to be created,
losing any in-progress service provider authentication, and not
redirecting back to the SP after a successful transaction.

If we hit the 401 page and there's a current transaction, add the
transaction ID to the link back to the login page, so the current
transaction is preserved.

Signed-off-by: Howard Johnson merlin@merlinthp.org

puiterwijk commented on line 5 of ipsilon/util/errors.py 7 years ago

This function will throw a 401 of itself if there is no transaction or if it has expired, which will get us in a loop.

rebased

7 years ago

merlinthp commented 7 years ago

You're no fun ;)

Fixed.

puiterwijk commented 7 years ago

Looks good to me, thanks!

puiterwijk commented 7 years ago

As discussed on IRC: after testing further, I think you will want "{% if tid %}", since if there is no tid value provided, "tid is not none" does evaluate to True.

rebased

7 years ago

merlinthp commented 7 years ago

Commit ec5ed54 fixes this pull-request

Pull-Request has been merged by merlin@merlinthp.org

7 years ago