Adding end to end tests and doc for the new validation #3077
Conversation
...crosoft.IdentityModel.JsonWebTokens.Tests/JsonWebTokenHandler.ValidateTokenAsyncTests_e2e.cs
Show resolved
Hide resolved
| // IDX10500: Signature validation failed. No security keys were provided to validate the signature. | ||
|
|
||
|
|
||
| // TODO: this message is not right. A security key was provided. but not the right one. |
There was a problem hiding this comment.
There was a problem hiding this comment.
I'll see to add a better worded message for this scenario.
| Assert.Contains("IDX10211", validationResult.Error.Message); | ||
| // IDX10211: Unable to validate issuer. The 'issuer' parameter is null or whitespace. | ||
|
|
||
| // This message needs to be updated to specifiy that it was not specified in the ValidationParameters? |
There was a problem hiding this comment.
There was a problem hiding this comment.
From what I can tell, this is a similar situation to the missing audience, with the exception that TokenValidationParameters does not have a RequireIssuer flag.
Based on the RFC, iss is an optional claim, and we could still default RequireIssuer to true, but to comply we should not force it to be required.
I can see to add the flag to ValidationParameters and update the issuer validation code.
We would need to decide if we want to backport this behaviour to TokenValidationParameters.
| Assert.IsType<AudienceValidationError>(validationResult.Error); | ||
| Assert.Contains("IDX10206", validationResult.Error.Message); | ||
| // IDX10206: Unable to validate audience. The 'audiences' parameter is empty. | ||
| // This message needs to be updated to specifiy that it was not specified in the ValidationParameters? |
There was a problem hiding this comment.
There was a problem hiding this comment.
From what I can tell, we seem to have removed the parameter RequireAudience in ValidationParameters, even though the aud claim is optional, failing if it is not present.
I'll see to bring it back and check it as part of the audience validation.
There was a problem hiding this comment.
I think ValidAudience is enough, @iNinja ?
no need to have requireAudience?
...crosoft.IdentityModel.JsonWebTokens.Tests/JsonWebTokenHandler.ValidateTokenAsyncTests_e2e.cs
Show resolved
Hide resolved
* Adding end to end tests * Update (cherry picked from commit 6df18f5)
…l from the feature branch (#3100) * Adding end to end tests and doc for the new validation (#3077) * Adding end to end tests * Update (cherry picked from commit 6df18f5) * Added new error messages for signature validation cases that were not accurately explained in the previous. * Updated tests failing after error message changes. --------- Co-authored-by: Jean-Marc Prieur <jmprieur@microsoft.com>
end to end tests and doc for the new validation
The goal here is to understand the public API and the developer experience.
ValidationParameters and their delegates
ValidationErrors