Bug 17481 - Cas Logout: bug 11048 has been incorrectly merged
Summary: Cas Logout: bug 11048 has been incorrectly merged
Status: Pushed to Master
Alias: None
Product: Koha
Classification: Unclassified
Component: Authentication (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low major (vote)
Assignee: Matthias Meusburger
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 18046
  Show dependency treegraph
 
Reported: 2016-10-21 15:03 UTC by Matthias Meusburger
Modified: 2017-02-09 08:23 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


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
Details | Diff | Splinter Review

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 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 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 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.