Bug 17481 - Cas Logout: bug 11048 has been incorrectly merged
Summary: Cas Logout: bug 11048 has been incorrectly merged
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Authentication (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low major
Assignee: Matthias Meusburger
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 18046
  Show dependency tree / graph
 
Reported: 2016-10-21 15:03 UTC by Matthias Meusburger
Modified: 2019-06-27 09:24 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:

Attachments
Bug 17481: Fix incorrect merge of bug 11048 (logout redirection for CAS authentication) (769 bytes, patch)
2016-10-21 15:06 UTC, Matthias Meusburger 		Details | Diff | Splinter Review
[PASSED QA] Bug 17481: Fix incorrect merge of bug 11048 (logout redirection for CAS authentication) (840 bytes, patch)
2016-11-07 10:20 UTC, Martin Renvoize (ashimema) 		Details | Diff | Splinter Review
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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.