gh-8589 Additional Jwt validation debug messages

[Budlee]

Adding extra logging for JWT Validation
#8589

[jzheaux]

Thanks, @Budlee! Before I review, can you tell me what this PR gives you that printing the stack trace in AuthenticationWebFilter would not give you? The reason I ask is that logging the stack trace is something that I'm thinking about adding.

[Budlee]

Hey @jzheaux, I think that would be good to have as well.
The stack trace for reactive would be horriblly long is my only thought. But as a trace maybe a good idea.

The other thing is in this specific case is that I have logging for each JWT Validator that fails. This is quite nice in the log.

Maybe it should change but the current validate gets the first validation fail even if there is many. So if we had the stach trace you would only see the first error of the token
org.springframework.security.oauth2.jwt.NimbusReactiveJwtDecoder#validateJwt

if ( result.hasErrors() ) {
			String message = result.getErrors().iterator().next().getDescription();
			throw new JwtValidationException(message, result.getErrors());
		}

This seems silly as the DelegatedOauth collects all
org.springframework.security.oauth2.core.DelegatingOAuth2TokenValidator#validate

for (OAuth2TokenValidator<T> validator : this.tokenValidators) {
			errors.addAll(validator.validate(token).getErrors());
		}

Also I think this result.getErrors().iterator().next().getDescription(); is an NPE waiting to happen

[jzheaux]

Thanks, @Budlee! I've left some feedback inline.

[jzheaux]

That's great feedback, @Budlee. I'll try and address a couple of your comments:

This seems silly as the DelegatedOauth collects all org.springframework.security.oauth2.core.DelegatingOAuth2TokenValidator#validate

What would you supply as the message for the exception if not the first validation error message?

Also I think this result.getErrors().iterator().next().getDescription(); is an NPE waiting to happen

I agree that this could be polished.

The OAuth2TokenValidatorResult constructor disallows null or empty lists, but it doesn't check if Collection<OAuth2Error> contains null elements. It might be good to add that assertion. Would you like to add a ticket to clean that up?

[Budlee]

@jzheaux,
I've resolved the issues that you had and i've add the tidy up for that potential NPE.

As the validators are now all logging it does make sense for just the first error to go back

Please let me know if you want any more changes

[jzheaux]

Thanks, @Budlee! I've left a bit more feedback.

[Budlee]

@jzheaux i've updates

[jzheaux]

Thanks for the updates, @Budlee!

It looks like the build is failing. Will you try rebasing, and double-check by running ./gradlew check? If there are any checkstyle errors, please fix them.

Then, at that point in preparation for merging, would you please squash your commits and format the commit message.

[jzheaux]

Thanks, @Budlee! This is now merged into master via b272805 and 90e5f45.

Note that I separated your two contributions into two separate commits for easier backporting. First, the additional logging and second the NPE polish.