Current Behavior
Currently, when attempting to work withz invalid client ID, an ERROR is logged : Authorization Request failed: java.lang.IllegalArgumentException: Invalid Client Registration with Id: xxxl
(org.springframework.security.oauth2.client.web.DefaultOAuth2AuthorizationRequestResolver#resolve(javax.servlet.http.HttpServletRequest, java.lang.String, java.lang.String))
Desired Behavior
Avoid ERRORs in logs for this case (either setup or altogether - switch to warning maybe?)
Context
When a security/penetration scan is run on the app, many errors like this are logged, but do not represent any actual problem with the app. All the errors are just a result of the scan. Therefore it would be nice if this error could be silenced - maybe changed to warning.
Thank you.
Comment From: jgrandja
@PunchyRascal There is only one log statement in OAuth2AuthorizationRequestRedirectFilter. Have you considered trying to turn off logging during penetration tests? For example:
logging.level.org.springframework.security.oauth2.client.web.OAuth2AuthorizationRequestRedirectFilter=OFF
Comment From: PunchyRascal
Since the pentest is a regular occurrence, I think that would not be sustainable - to keep turning this off and on again and releasing to prod.
Comment From: jgrandja
@PunchyRascal A correctly configured application would not send an invalid client registration id. I understand your pentest is a different testing scenario but it doesn't make sense to "convenience" a pentest scenario as this would also "convenience" a malicious request that was attempting to guess a valid registration id. I would prefer to be disruptive to a malicious request by keeping this existing behaviour and throwing an exception. Makes sense?
Comment From: PunchyRascal
I could argue that protection against brute force attacks is not in the scope of this module, if that is what you're saying, but hey - who am I? If you do not think this is doable, even via settings, we'll have to manage otherwise 🙂
Comment From: jgrandja
@PunchyRascal
I could argue that protection against brute force attacks is not in the scope of this module, if that is what you're saying
No that's not what I'm saying. As mentioned in previous comment
A correctly configured application would not send an invalid client registration id
An invalid client registration id in the request is an error in the calling application, hence, the signal of the error condition to the caller so they can correct.
Comment From: PunchyRascal
This request is about log messages being generated. I am working under the assumption, that the caller does not have access to the Oauth app's logs.
Comment From: jgrandja
Apologies @PunchyRascal, it looks like I misinterpreted the request. If it's as simple as changing the log message from error to warn for invalid client registration id scenarios then we can apply the enhancement. Please confirm if this is exactly what you're looking for.
Comment From: PunchyRascal
Yes, that would be great.
Comment From: sjohnr
Awesome, thanks @PunchyRascal!
I do want to point out for future users encountering this issue that this change (logging invalid client registration ID at WARN level) will still require many applications to tune their logs to get rid of this warning. However, it will allow this message to be turned off permanently if desired while still retaining other errors in the logs.