[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: Text File magnolia-module-cas-second.patch     Text File magnolia-module-cas.patch    
Issue Links:
causality
is causing MGNLCAS-20 Do not invalidate session if user vis... Closed
relation
is related to MGNLCAS-17 Update LDAP dependency to 1.6.3 Closed
is related to MGNLCAS-15 Dynamically generate CAS service url ... Closed
is related to MGNLCAS-16 Append service url to logout url Closed
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:

  • Tests failed, module no longer compiled under 5.4.
  • Under 5.4, logout did not work unless gzip filter enabled (gzip masks the problem by wrapping the response object).
  • Under 5.4, attempting to load an image on an unauthorized path (including non-existent paths) while logged in via CAS causes fatal “Communication Error” preventing all editing.
  • Users who do not yet have permission to the system got an ugly error message.
  • Users who do not yet have permission to the system had to manually clear cookies after getting permission added.
  • Usernames were not case-insensitive. (We changed it to lower-case the username as received from CAS).

All of these issues are now resolved.

The patch file also includes the patches we submitted in MGNLCAS-15, MGNLCAS-16, and MGNLCAS-17, so if accepted, those can all be closed.

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
Rich

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:
new caseInsensitiveUserNames property in info.magnolia.cms.security.cas.CASModule has wrong name because of isCaseSensitiveUserNames method. Please rename it to caseSensitiveUserNames and set it by default to true

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 MGNLCAS-20 to address this issue.

Best regards
Roman

Generated at Sun Feb 11 23:59:01 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.