Fix over-reporting of IDX14100 stating "there are no dots" #2618
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
IDX14100 makes sense when it is thrown from JsonWebToken constructor or Read method when there is really one dot, but not as a generic error message any time there is a token validation failure for any reason.
azure-activedirectory-identitymodel-extensions-for-dotnet/src/Microsoft.IdentityModel.JsonWebTokens/JsonWebToken.cs
Lines 415 to 417 in 7cdeadb
The above line seems to be the only correct usage of the IDX14100, and it remains after this PR. IDX14120 gives a more precise error message when a "JWT is not well formed, there is only one dot", but that winds up getting wrapped in another exception with the incorrect IDX14100 message and relogged by
JsonWebTokenHandler.ReadToken
. Other validation errors like a non-numeric "iat" claim also result get the incorrect IDX14100 message.This exception wrapping by
JsonWebTokenHandler.ReadToken
seems to serve no purpose other than make it appear that the token is invalid for the wrong reason. The original reporter pointed out that this was obscuring an exception caused by deleting System.Buffers.dll. While this isn't something a developer should do, it demonstrates why wrapping arbitrary exceptions is bad in this scenario. Even if the issue really was what is described by IDX14100,JsonWebTokenHandler.ReadToken
still unnecessary wraps that with an identical outer exception and logs it a second time.The change to use a custom
ArgumentException
rather than an IDX14100SecurityTokenMalformedException
whenJsonWebTokenHandler.ValidateTokenAsync
gets a non-JsonWebToken
SecurityToken
is less important, but it is another place IDX14100 is being used that has nothing to do with the number of dots in the token. I don't think we should be logging ArgumentExceptions at all, but I continue logging in order to stick to the conventions of the library. If we make a change to not log ArgumentExceptions, we should probably do that everywhwere.Fixes #2058