Skip to content
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

CasAuthenticationFilter.successfulAuthentication missing call to securityContextRepository.saveContext #13243

Closed
tokarls opened this issue May 30, 2023 · 3 comments · Fixed by #13276
Assignees
Labels
in: cas An issue in spring-security-cas type: bug A general bug
Milestone

Comments

@tokarls
Copy link

tokarls commented May 30, 2023

Describe the bug
org.springframework.security.cas.web.CasAuthenticationFilter.successfulAuthentication seems to be missing a call to securityContextRepository.saveContext as per the Spring 6 migration document. I think this makes the filter completely unusable on Spring6/Boot3 and begs to question if the CasAuthenticationFilter code is production ready for Spring 6 otherwise.

To Reproduce
Use the filter

Expected behavior
SecurityContext is saved between requests :)

@tokarls tokarls added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels May 30, 2023
@marcusdacoregio marcusdacoregio self-assigned this May 30, 2023
@marcusdacoregio marcusdacoregio added in: cas An issue in spring-security-cas and removed status: waiting-for-triage An issue we've not yet triaged labels May 30, 2023
@marcusdacoregio marcusdacoregio added this to the 6.1.1 milestone May 30, 2023
@Anubhav-2000
Copy link
Contributor

Hi @marcusdacoregio, can i try this?

marcusdacoregio added a commit to marcusdacoregio/spring-security that referenced this issue Jun 1, 2023
@marcusdacoregio
Copy link
Contributor

Hi @Anubhav-2000, yes, the issue is yours.

Ideally, I think we should keep a reference of the SecurityContextRepository set in the constructor and call it in the successfullAuthentication method. Something like this:

private SecurityContextRepository securityContextRepository = new HttpSessionSecurityContextRepository();;

public CasAuthenticationFilter() {
	super("/login/cas");
	setAuthenticationFailureHandler(new SimpleUrlAuthenticationFailureHandler());
	setSecurityContextRepository(this.securityContextRepository);
}

// ...

@Override
protected final void successfulAuthentication(HttpServletRequest request, HttpServletResponse response,
		FilterChain chain, Authentication authResult) throws IOException, ServletException {
	// ...
	SecurityContext context = SecurityContextHolder.createEmptyContext();
	context.setAuthentication(authResult);
	SecurityContextHolder.setContext(context);
	this.securityContextRepository.saveContext(context, request, response);
	// ...
}

Then, in 6.2, we can override the setter method and update both references.

@Anubhav-2000
Copy link
Contributor

Thanks @marcusdacoregio , i have not overridden the setter method and done the rest of the changes.

marcusdacoregio added a commit that referenced this issue Jun 5, 2023
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: bug A general bug
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants