Skip to content

Restore CAS module and update it for cas-client-core 4.0.0 #12362

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

Closed
wants to merge 8 commits into from

Conversation

hdeadman
Copy link
Contributor

This is a quick attempt at addressing #11674 and #12304. It reverts the CAS module deletion in one commit, then adjusts the packages to the new package structure in the cas-client-core module and changes Jasig references to Apereo, and then in another commit it changes CAS module to use its own AuthenticationToken for processing the service ticket rather than storing it on UsernamePasswordAuthenticationToken.

I copied UsernamePasswordAuthenticationToken and called it CasServiceTicketAuthenticationToken and then change the principal which was repurposed as a user agent type (stateless or stateful) to use an enum with the constant values used to tell spring security whether to cache the validated service ticket or not.

I haven't tested this in an actual webapp yet.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Dec 10, 2022
@hdeadman
Copy link
Contributor Author

hdeadman commented Dec 11, 2022

I did test this with an app updated to spring-security 6.x. After dealing with some deprecated spring-security stuff that I was using I was getting into a redirect loop b/c my CasAuthenticationFilter wasn't using:

setSecurityContextRepository(new HttpSessionSecurityContextRepository());

Once I set that on my CasAuthenticationFilter bean, I was able to login via CAS. It needed that in order for the Authentication to be available after redirect to the URL requested before the redirect to CAS. Maybe the CasAuthenticationFilter should set the repository by default, or setSecurityContextRepository(new DelegatingSecurityContextRepository( new RequestAttributeSecurityContextRepository(), new HttpSessionSecurityContextRepository()));.

I also noticed that when it was in a redirect loop, the HttpSessionRequestCache was adding the matchingRequestParameterName of "continue" over and over after each redirect, e.g. continue&continue&continue...

@marcusdacoregio marcusdacoregio added this to the 6.1.0-M1 milestone Dec 13, 2022
@marcusdacoregio marcusdacoregio added in: cas An issue in spring-security-cas type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Dec 13, 2022
@hdeadman
Copy link
Contributor Author

hdeadman commented Jan 6, 2023

Let me know if there is anything I can do to help this get merged. It would be nice to get it in the snapshots so people could try it out before the release.

@marcusdacoregio
Copy link
Contributor

marcusdacoregio commented Jan 12, 2023

Thanks @hdeadman, we'll look into that as soon as we get the priorities sorted out for the release.

@marcusdacoregio marcusdacoregio modified the milestones: 6.1.0-M1, 6.1.0-M2 Jan 16, 2023
@TheNasCrazy
Copy link

Hello,
Need to know when this PR will be merged and integrate for the next release ?

@marcusdacoregio marcusdacoregio self-assigned this Feb 16, 2023
Copy link
Contributor

@marcusdacoregio marcusdacoregio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @hdeadman for your work on this. I have some considerations before merging it:

  • The copyright notice that contains Acegi we leave them as-it-is, so please drop this commit
  • Please ensure that the other copyright notices are updated to the current year
  • Can you ensure that we are using the latest dependency version? I see that there was a release on Jan 16th.
  • We need to improve the documentation by adding code snippets using Java Configuration, are you able to do that? If not, I can create a ticket and we can work on that later on.

If the class is CasServiceTicketAuthenticationToken then the enum forces the agent type to be one of two things this if is checking
@hdeadman
Copy link
Contributor Author

I dropped the commit with the Acegi copyright changes, plus removed one Acegi copyright change that was in another commit, and I updated the java cas-client-core dependency to 4.0.1. I updated all the non-Acegi copyright years under cas module to include 2023, not sure if that was what you meant but it's in one commit so I can drop that if its wrong.

I will try to work on documentation updates this weekend. I might have to add a cas client app to spring-security-samples so I put in examples that work.

@marcusdacoregio
Copy link
Contributor

Thanks @hdeadman, I've opened #12691 to revisit the documentation, therefore you can keep the documentation efforts in another PR and we can discuss how we want the documentation to look like in that particular issue.

@marcusdacoregio
Copy link
Contributor

Great work on the PR @hdeadman. I've applied some polish on top of your commits, you can see them here. This is looking good and almost ready to be merged, I'm talking to @rwinch to see if there are other steps before merging this.

@marcusdacoregio
Copy link
Contributor

Closed via b4d3ac6, e0284a4 and 04369cf

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: cas An issue in spring-security-cas type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants