-
Notifications
You must be signed in to change notification settings - Fork 5.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SEC-2547: Update to cas-client-core-3.3.3 #2759
Comments
Rob Winch said: Thanks for the PR. I am in the middle of a few other things, but I will take a look & merge this sometime next week. |
Rob Winch said: This has been merged into master and 3.2.x. Thanks again for the PR :) |
Rob Winch said: Due to passivity problems when updating this has been reverted. I have updated the issue description to describe what needs to be fixed in CAS before we update. |
Hans-Joachim Kliemeck said: hey, thank you for your comments. i added some code that the version change to 3.3.1 is compatible: #86 . with this pull request the issue 224 and 225 is addressed, but 223 is not:
why do you think its not participating in the lifecycle? the init method of SingleSignOutHandler is called by the init method of SingleSignOutFilter (that is part of javax.servlet.Filter) |
Rob Winch said:
Thanks. Unfortunately we won't be able to upgrade until the passivity issues are fixed. The reason for this is that it will potentially break users that are using the non-passive APIs. Furthermore, if they are interested in upgrading the CAS client it is very simple for them to declare the updated version.
To illustrate the problem, you can invoke "./gradlew build" from the root of the project. You will notice that the NullPointerException occurs when using the updated dependency versions. If you use the current version, it does not have any issues. |
Hans-Joachim Kliemeck said: hey, i dont know how to differenciate between "passive" / "non passive api's", but maybe its only an issue of translation.
they are both not directly used on real applications that are based on spring security. what do you think? |
Hans-Joachim Kliemeck said: regarding CASC-224+CASC-225: regarding CASC-223: |
Rob Winch said:
In regards to what I view as "passive" / "non passive api's" traditionally I would say that anything that is public scope should be passive unless clearly indicated that it is an internal API. I could possibly be convinced that the CommonUtils does not need to be corrected before resolving this issue. However, I think it is quite likely that users with CAS Proxy Tickets and Spring Security are constructing a TicketValidator instance and injecting it on an instance of AbstractTicketValidationFilter. This is due to the fact that in a Spring environment user's are likely invoking getter and setters and not relying on Filter#init(FilterConfig) methods.
Perhaps I shouldn't have been quite so prescriptive. The main issue is that a NullPointerException occurs after updating from cas-client-3.1.12. It is true that the commit you mentioned wasn't an issue in 3.1.12. However, the introduction of 477fc582f044 assumes the code in a947490c0 is invoked. Since it is not invoked the result is a NullPointerException. In short, the main concern here is to resolve the NullPointerException for those that create a Filter within a Spring container. Once that is fixed, then we can resume upgrading the CAS client. |
Rob Winch said: Moved to 4.0.x backlog until fixes are available in CAS |
Hans-Joachim Kliemeck (Migrated from SEC-2547) said:
Since the versions across the project are inconsistent, i took the possibility and made them consistent + upgraded the version.
pull request: #80
The following issues need fixed before we can update
The text was updated successfully, but these errors were encountered: