[MGNLCAS-18] Laundry list of CAS fixes Texas State has implemented Created: 26/Jan/16 Updated: 10/Mar/16 Resolved: 10/Mar/16 |
|
| Status: | Closed |
| Project: | Central Authentication Service |
| Component/s: | None |
| Affects Version/s: | 1.1.1 |
| Fix Version/s: | 1.3 |
| Type: | Improvement | Priority: | Major |
| Reporter: | Nickolaus Wing | Assignee: | Roman Kovařík |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | support | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Attachments: |
|
||||||||||||||||||||||||
| Issue Links: |
|
||||||||||||||||||||||||
| Template: |
|
||||||||||||||||||||||||
| Patch included: |
Yes
|
||||||||||||||||||||||||
| Acceptance criteria: |
Empty
|
||||||||||||||||||||||||
| Task DoD: |
[ ]*
Doc/release notes changes? Comment present?
[ ]*
Downstream builds green?
[ ]*
Solution information and context easily available?
[ ]*
Tests
[ ]*
FixVersion filled and not yet released
[ ] 
Architecture Decision Record (ADR)
|
||||||||||||||||||||||||
| Testcase included: |
Yes
|
||||||||||||||||||||||||
| Date of First Response: | |||||||||||||||||||||||||
| Sprint: | Kromeriz 32 | ||||||||||||||||||||||||
| Story Points: | 5 | ||||||||||||||||||||||||
| Description |
|
We upgraded from Mgnl 4.5.24 to 5.4.2 on Dec 16, 2015. During the course of that upgrade we found a number of medium to major issues with the CAS module and resolved them. We are pushing out another release Feb 9 with a few more changes that are also included here. None of them are specific to our institution, they are pretty much just straight up improvements and fixes. I'll try to enumerate the things that were broken before we started:
All of these issues are now resolved. The patch file also includes the patches we submitted in Note: I just noticed that I didn't fix the delta label in the versionhandler class. All those tasks should be attached to 1.1.2 since that's your next version. |
| Comments |
| Comment by Nickolaus Wing [ 07/Feb/16 ] |
|
We uncovered a regression with the "loading forbidden image causes communication error" problem. This patch, applied after the big patch that opened the ticket, fixes it. |
| Comment by Richard Gange [ 07/Feb/16 ] |
|
Hi nwing. Thanks for this all this. AFAIK we don't have any plans to continue support for CAS. There's just so many other higher priority projects at this time. Might I suggest that you package this module up and host it at GitHub... |
| Comment by Nickolaus Wing [ 07/Feb/16 ] |
|
We can do that. We would release as gpl, no enterprise. I presume your suggestion means that is OK? |
| Comment by Richard Gange [ 08/Feb/16 ] |
|
Hi nwing I think I may have mispoke. I did not realize that CAS was of the EE type. I made the silly assumption that it was CE. Sorry about that. After speaking with some others at the company, in order to allow you to fork it we would need to change the licensing. It doesn't appear that's something we want to do. So instead what we would like to do is gather your code fixes and apply it to a new version of the module. To get the ball rolling I would open a support request on your behalf. Then we will link the support ticket to the bug report and go from there. Again thanks for this fix and sorry about the confusion. Cheers |
| Comment by Roman Kovařík [ 22/Feb/16 ] |
|
CASSingleSignOutFilter: it should be possible to use out-of-the-box info.magnolia.cms.filters.FilterDecorator instead. |
| Comment by Milan Divilek [ 25/Feb/16 ] |
|
Reopen: info.magnolia.cms.security.cas.CASModule#createLogoutURL is not used at all, please remove it |
| Comment by Nickolaus Wing [ 10/Mar/16 ] |
|
I've just reviewed all these changes, and they look good, except that the issue resolved by my second patch file is still an issue in your version. To fix it, remove the session invalidation in CASClientCallback.handleUnauthorizedUser(). If we invalidate the session at that point, and the unauthorized URL is an element on the page, like an image, it will disrupt our Vaadin communications and they get the dreaded "Communication Error". |
| Comment by Roman Kovařík [ 10/Mar/16 ] |
|
Hello Nickolaus, thanks for checking. I've just closed this ticket since it's already realeased as version 1.3 and created the follow up Best regards |