Log that a token can not be introspected if the introspection address is null #18976
Conversation
| @@ -144,6 +144,13 @@ public TokenVerificationResult verifyJwtToken(String token) throws InvalidJwtExc | |||
| } | |||
|
|
|||
| public Uni<TokenVerificationResult> introspectToken(String token) { | |||
| if (client.getMetadata().getIntrospectionUri() == null) { | |||
| LOG.debugf( | |||
There was a problem hiding this comment.
Shouldn't this message be part of the AuthenticationFailedException below?
There was a problem hiding this comment.
Hi George @gastaldi, sorry, I did not mean to take a lot of your time :-)
In a good number of cases quarkus-oidc would only log and throw the exception to prevent the double logging or the message escaping to the client, but in a few cases it would log and propagate the same message (for the security exception it is preferred); but it all is not consistent :-).
As far as I recall this exception's message is not logged at the HttpSecurityRecorder level, hence we log and throw it without including the message (in most cases I believe).
Are you OK with this explanation for now ? It is not an urgent PR so we can keep discussing whenever you have time :-)
There was a problem hiding this comment.
That's fine.
My only concern is that this message won't be visible to anyone unless debug level is enabled. Perhaps it should be logged as a warning to facilitate the investigation?
There was a problem hiding this comment.
Hi @gastaldi Right, all verification issues are logged at the debug level in quarkus-oidc - I believe that was @stuartwdouglas's preference as well (as far as I recall from the earlier discussions) - if we log it at WARNING then in a system which deals with the binary tokens requiring the introspection every token request will log this warning and can flood it.
That said, the fact that no introspection path is available may warrant a warning.
FYI, the remote introspection is used for all non-JWT tokens and as a recovery option if the local JWT verification fails - so in this latter case the warning that no introspection path is available is not really a warning - a few providers like Auth0 or Google won't provide it and if JWT verification with the local key fails (which is logged at the debug level) then this recovery attempt will cause the warning - where Quarkus just tries to check the already failed token remotely just in case.
So I'm hesitating :-), @stuartwdouglas - what is your preference, should we keep the security related log message at a DEBUG level or raise it in some cases like this one to WARNING ?
thanks
There was a problem hiding this comment.
That makes sense. Go ahead and merge this and let's see how it goes then 👍🏻
There was a problem hiding this comment.
Hi George @gastaldi OK, thanks for the time; we should have a wider discussion at the dev forum, it will be useful to discuss for sure :-)
I've got 500 with a long NPE when trying to verify an Auth0 access token in a binary format from Dev UI - since Auth0 does not have an introspection endpoint and JWKs can be used in this case.