Bug 17481

Summary:	Cas Logout: bug 11048 has been incorrectly merged
Product:	Koha	Reporter:	Matthias Meusburger <matthias.meusburger>
Component:	Authentication	Assignee:	Matthias Meusburger <matthias.meusburger>
Status:	CLOSED FIXED	QA Contact:	Testopia <testopia>
Severity:	major
Priority:	P5 - low	CC:	dpavlin, jonathan.druart, julian.maurice, kyle, martin.renvoize, mtj
Version:	unspecified
Hardware:	All
OS:	All
Change sponsored?:	---	Patch complexity:	---
Documentation contact:		Documentation submission:
Text to go in the release notes:		Version(s) released in:
Circulation function:
Bug Depends on:
Bug Blocks:	18046
Attachments:	Bug 17481: Fix incorrect merge of bug 11048 (logout redirection for CAS authentication) [PASSED QA] Bug 17481: Fix incorrect merge of bug 11048 (logout redirection for CAS authentication)

Description Matthias Meusburger 2016-10-21 15:03:57 UTC

Bug 11048 (Fix logout redirection for CAS authentication) has been incorrectly merged:

In Bugzilla, https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=23865 :

-    print $query->redirect( $cas->logout_url($uri));
+    print $query->redirect( $cas->logout_url(url => $uri));

In master, git show c251f752 : 

     print $query->redirect( $cas->logout_url($uri));
+    print $query->redirect( $cas->logout_url(url => $uri));


=> CAS redirection is still broken in master.

Comment 1 Matthias Meusburger 2016-10-21 15:06:31 UTC

Created attachment 56723 [details] [review]
Bug 17481: Fix incorrect merge of bug 11048 (logout redirection for CAS authentication)

Comment 2 Katrin Fischer 2016-11-01 23:14:03 UTC

Should this be 'Needs Signoff' or something else to get RMs attention?

Comment 3 Jonathan Druart 2016-11-02 14:52:51 UTC

Looking at the changes, it sounds trivial that the patches waited for 6 months and that it conflicts.
Redirect after a redirect does not make sense.
I would be keen to sent it to PQA directly

Comment 4 Katrin Fischer 2016-11-02 14:57:49 UTC

Go for it!

Comment 5 Martin Renvoize (ashimema) 2016-11-07 10:13:52 UTC

I'd happily PQA this directly too.. that's three of us n agreement.. I'm gonna just go ahead and set it ;)

Comment 6 Martin Renvoize (ashimema) 2016-11-07 10:20:39 UTC

Created attachment 57248 [details] [review]
[PASSED QA] Bug 17481: Fix incorrect merge of bug 11048 (logout redirection for CAS authentication)

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Comment 7 Kyle M Hall (khall) 2016-11-07 16:35:11 UTC

Pushed to master for 16.11, thanks Matthias!

Comment 8 Julian Maurice 2016-11-30 12:09:16 UTC

Pushed to 3.22.x, will be in 3.22.13

Comment 9 Mason James 2016-12-14 12:02:24 UTC

Pushed to 16.05.x, for 16.05.06 release

Comment 10 Jonathan Druart 2017-02-09 07:56:12 UTC

Matthias, Has this patch been tested?

It seems that it still does not work, see bug 18046.
This story is becoming to be very too long...

Comment 11 Matthias Meusburger 2017-02-09 08:00:12 UTC

Hi,

I did test this patch back then. It worked perfectly.

Something has changed since, obviously.

Currently waiting for a CAS server to test this against master.

Comment 12 Katrin Fischer 2017-02-09 08:23:05 UTC

I was wondering how to solve this. Coudl one solution be to send the user back to OpacBaseUrl? It seems to make sense to send them back to the start page on logout.