Skip to content

Add logging to CsrfTokenRequestHandler implementations #13626

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.

Sign up for GitHub

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

Jump to bottom
Open
jzheaux opened this issue Aug 7, 2023 · 6 comments · May be fixed by #16994
Open

Add logging to CsrfTokenRequestHandler implementations #13626

jzheaux opened this issue Aug 7, 2023 · 6 comments · May be fixed by #16994
Assignees
@jzheaux @yybmion
Labels
in: web An issue in web modules (web, webmvc) type: enhancement A general enhancement

Comments

@jzheaux
Copy link
Contributor

jzheaux commented Aug 7, 2023 •
edited
Loading

It would be helpful to have logging to show what logical path CsrfTokenRequestHandler implementations are taking to read from and write tokens to the request.

For example, in resolveCsrfTokenValue it would likely be helpful to know where it found the csrf token (header or parameter). In handle it would be helpful to know the name of the request attribute used to write the token to the request.

Following this pattern, it would additionally be helpful in XorCsrfTokenRequestAttributeHandler to log when the method fails to find a token value and thus returns null. For example if decoding fails:

try {
	actualBytes = Base64.getUrlDecoder().decode(actualToken);
}
catch (Exception ex) {
	return null;
}

It would be nice to log that we are returning null since decoding failed:

try {
	actualBytes = Base64.getUrlDecoder().decode(actualToken);
}
catch (Exception ex) {
+	this.logger.trace("Failed to find CSRF token since Base64 decoding failed", ex);
	return null;
}
@jzheaux jzheaux added status: waiting-for-triage An issue we've not yet triaged type: enhancement A general enhancement in: web An issue in web modules (web, webmvc) and removed status: waiting-for-triage An issue we've not yet triaged labels Aug 7, 2023
@andreilisa
Copy link

Hello @jzheaux, I would like to work on it if it is actual, but I will need some details.
What information should be logged ?

@yybmion
Copy link

yybmion commented Mar 29, 2025

Hi @jzheaux , I'm currently working on the issue to add logging to CsrfTokenRequestHandler implementations. Could you confirm if modifying only these two classes (CsrfTokenRequestAttributeHandler and XorCsrfTokenRequestAttributeHandler) is sufficient to complete this task? Are there any other implementations I should be aware of?

@jzheaux
Copy link
Contributor Author

jzheaux commented Apr 1, 2025

Thanks for volunteering, @yybmion! I've updated the description with some additional information.

@jzheaux
Copy link
Contributor Author

jzheaux commented Apr 1, 2025

@andreilisa, I realize it's been quite some time; still I wanted to reach out and apologize for missing your original offer. If you are still interested in helping, please join me in the review process when the PR is ready.

@andreilisa
Copy link

@jzheaux, all good.
Sure, I will join the review process. Thank you :)

@yybmion
Copy link

yybmion commented Apr 1, 2025

@jzheaux Thank you! I've checked the updated description.

@yybmion yybmion linked a pull request Apr 24, 2025 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jzheaux jzheaux

@yybmion yybmion

Labels
in: web An issue in web modules (web, webmvc) type: enhancement A general enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add logging to CsrfTokenRequestHandler implementations yybmion/spring-security
3 participants
@jzheaux @andreilisa @yybmion