-
Notifications
You must be signed in to change notification settings - Fork 411
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Updated documentation for the new validation model and restructured internals #3056
Open
iNinja
wants to merge
11
commits into
dev
Choose a base branch
from
iinglese/tidy-up-new-validation-model
base: dev
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 4 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
a664f72
Removed static stack frames and replaced with the simplified approach…
iNinja 9b66954
Updated IssuerValidationSource to be extensible. Extracted validated …
iNinja 073aae5
Updated documentation
iNinja 143daa7
Merge branch 'dev' into iinglese/tidy-up-new-validation-model
iNinja 206c0be
Added nullability annotations to ValidationParameters. Enabled settin…
iNinja aa94abc
Handle case where ValidateActor is true, there is an actor token, but…
iNinja 9e2746c
Updated documentation, added missing interfaces and methods required …
iNinja 0ba56b4
Merge branch 'dev' into iinglese/tidy-up-new-validation-model
iNinja 42a41bc
Merge branch 'dev' into iinglese/tidy-up-new-validation-model
iNinja 54063b3
Added missing documentation around validation errors
iNinja 5ac80f6
Added CLSCompliant flag to Log methods to address the build issue unt…
iNinja File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -3,7 +3,6 @@ | |||||||
|
||||||||
using System; | ||||||||
using System.Collections.Generic; | ||||||||
using System.Diagnostics; | ||||||||
using System.Linq; | ||||||||
using System.Text; | ||||||||
using Microsoft.IdentityModel.Logging; | ||||||||
|
@@ -31,49 +30,42 @@ internal ValidationResult<string> DecryptToken( | |||||||
{ | ||||||||
if (jwtToken == null) | ||||||||
{ | ||||||||
StackFrame tokenNullStackFrame = StackFrames.DecryptionTokenNull ??= new StackFrame(true); | ||||||||
return ValidationError.NullParameter( | ||||||||
nameof(jwtToken), | ||||||||
tokenNullStackFrame); | ||||||||
ValidationError.GetCurrentStackFrame()); | ||||||||
} | ||||||||
|
||||||||
if (validationParameters == null) | ||||||||
{ | ||||||||
StackFrame validationParametersNullStackFrame = StackFrames.DecryptionValidationParametersNull ??= new StackFrame(true); | ||||||||
return ValidationError.NullParameter( | ||||||||
nameof(validationParameters), | ||||||||
validationParametersNullStackFrame); | ||||||||
ValidationError.GetCurrentStackFrame()); | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
} | ||||||||
|
||||||||
if (string.IsNullOrEmpty(jwtToken.Enc)) | ||||||||
{ | ||||||||
StackFrame headerMissingStackFrame = StackFrames.DecryptionHeaderMissing ??= new StackFrame(true); | ||||||||
return new ValidationError( | ||||||||
new MessageDetail(TokenLogMessages.IDX10612), | ||||||||
ValidationFailureType.TokenDecryptionFailed, | ||||||||
typeof(SecurityTokenException), | ||||||||
headerMissingStackFrame); | ||||||||
ValidationError.GetCurrentStackFrame()); | ||||||||
} | ||||||||
|
||||||||
(IList<SecurityKey>? contentEncryptionKeys, ValidationError? validationError) result = | ||||||||
GetContentEncryptionKeys(jwtToken, validationParameters, configuration, callContext); | ||||||||
|
||||||||
if (result.validationError != null) | ||||||||
{ | ||||||||
StackFrame decryptionGetKeysStackFrame = StackFrames.DecryptionGetEncryptionKeys ??= new StackFrame(true); | ||||||||
return result.validationError.AddStackFrame(decryptionGetKeysStackFrame); | ||||||||
} | ||||||||
return result.validationError.AddCurrentStackFrame(); | ||||||||
|
||||||||
if (result.contentEncryptionKeys == null || result.contentEncryptionKeys.Count == 0) | ||||||||
{ | ||||||||
StackFrame noKeysTriedStackFrame = StackFrames.DecryptionNoKeysTried ??= new StackFrame(true); | ||||||||
return new ValidationError( | ||||||||
new MessageDetail( | ||||||||
TokenLogMessages.IDX10609, | ||||||||
LogHelper.MarkAsSecurityArtifact(jwtToken, JwtTokenUtilities.SafeLogJwtToken)), | ||||||||
ValidationFailureType.TokenDecryptionFailed, | ||||||||
typeof(SecurityTokenDecryptionFailedException), | ||||||||
noKeysTriedStackFrame); | ||||||||
ValidationError.GetCurrentStackFrame()); | ||||||||
} | ||||||||
|
||||||||
return JwtTokenUtilities.DecryptJwtToken( | ||||||||
|
@@ -211,7 +203,6 @@ internal ValidationResult<string> DecryptToken( | |||||||
return (unwrappedKeys, null); | ||||||||
else | ||||||||
{ | ||||||||
StackFrame decryptionKeyUnwrapFailedStackFrame = StackFrames.DecryptionKeyUnwrapFailed ??= new StackFrame(true); | ||||||||
ValidationError validationError = new( | ||||||||
new MessageDetail( | ||||||||
TokenLogMessages.IDX10618, | ||||||||
|
@@ -220,7 +211,7 @@ internal ValidationResult<string> DecryptToken( | |||||||
LogHelper.MarkAsSecurityArtifact(jwtToken, JwtTokenUtilities.SafeLogJwtToken)), | ||||||||
ValidationFailureType.TokenDecryptionFailed, | ||||||||
typeof(SecurityTokenKeyWrapException), | ||||||||
decryptionKeyUnwrapFailedStackFrame); | ||||||||
ValidationError.GetCurrentStackFrame()); | ||||||||
|
||||||||
return (null, validationError); | ||||||||
} | ||||||||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,6 @@ | |
|
||
using System; | ||
using System.Collections.Generic; | ||
using System.Diagnostics; | ||
using System.Linq; | ||
using System.Threading; | ||
using System.Threading.Tasks; | ||
|
@@ -18,22 +17,14 @@ public partial class JsonWebTokenHandler : TokenHandler | |
{ | ||
/// <summary> | ||
/// Validates a token. | ||
/// On a validation failure, no exception will be thrown; instead, the exception will be set in the returned TokenValidationResult.Exception property. | ||
/// Callers should always check the TokenValidationResult.IsValid property to verify the validity of the result. | ||
/// On a validation failure, no exception will be thrown; instead, the <see cref="ValidationError"/> will contain the information about the error that occurred. | ||
/// Callers should always check the ValidationResult.IsValid property to verify the validity of the result. | ||
/// </summary> | ||
/// <param name="token">The token to be validated.</param> | ||
/// <param name="validationParameters">The <see cref="ValidationParameters"/> to be used for validating the token.</param> | ||
/// <param name="callContext">A <see cref="CallContext"/> that contains useful information for logging.</param> | ||
/// <param name="callContext">A <see cref="CallContext"/> that contains call information.</param> | ||
/// <param name="cancellationToken">A <see cref="CancellationToken"/> that can be used to request cancellation of the asynchronous operation.</param> | ||
/// <returns>A <see cref="ValidationResult{TResult}"/> with either a <see cref="ValidatedToken"/> if the token was validated or an <see cref="ValidationError"/> with the failure information and exception otherwise.</returns> | ||
/// <remarks> | ||
/// <para>ValidationError.GetException() will return one of the following exceptions if the <paramref name="token"/> is invalid.</para> | ||
/// </remarks> | ||
/// <exception cref="ArgumentNullException">Returned if <paramref name="token"/> is null or empty.</exception> | ||
/// <exception cref="ArgumentNullException">Returned if <paramref name="validationParameters"/> is null.</exception> | ||
/// <exception cref="ArgumentException">Returned if 'token.Length' is greater than <see cref="TokenHandler.MaximumTokenSizeInBytes"/>.</exception> | ||
/// <exception cref="SecurityTokenMalformedException">Returned if <paramref name="token"/> is not a valid <see cref="JsonWebToken"/>, <see cref="ReadToken(string, CallContext)"/></exception> | ||
/// <exception cref="SecurityTokenMalformedException">Returned if the validationParameters.TokenReader delegate is not able to parse/read the token as a valid <see cref="JsonWebToken"/>, <see cref="ReadToken(string, CallContext)"/></exception> | ||
internal async Task<ValidationResult<ValidatedToken>> ValidateTokenAsync( | ||
string token, | ||
ValidationParameters validationParameters, | ||
|
@@ -42,31 +33,28 @@ internal async Task<ValidationResult<ValidatedToken>> ValidateTokenAsync( | |
{ | ||
if (string.IsNullOrEmpty(token)) | ||
{ | ||
StackFrame nullTokenStackFrame = StackFrames.TokenStringNull ??= new StackFrame(true); | ||
return ValidationError.NullParameter( | ||
nameof(token), | ||
nullTokenStackFrame); | ||
ValidationError.GetCurrentStackFrame()); | ||
} | ||
|
||
if (validationParameters is null) | ||
{ | ||
StackFrame nullValidationParametersStackFrame = StackFrames.TokenStringValidationParametersNull ??= new StackFrame(true); | ||
return ValidationError.NullParameter( | ||
nameof(validationParameters), | ||
nullValidationParametersStackFrame); | ||
ValidationError.GetCurrentStackFrame()); | ||
} | ||
|
||
if (token.Length > MaximumTokenSizeInBytes) | ||
{ | ||
StackFrame invalidTokenLengthStackFrame = StackFrames.InvalidTokenLength ??= new StackFrame(true); | ||
return new ValidationError( | ||
new MessageDetail( | ||
TokenLogMessages.IDX10209, | ||
LogHelper.MarkAsNonPII(token.Length), | ||
LogHelper.MarkAsNonPII(MaximumTokenSizeInBytes)), | ||
ValidationFailureType.InvalidSecurityToken, | ||
typeof(ArgumentException), | ||
invalidTokenLengthStackFrame); | ||
ValidationError.GetCurrentStackFrame()); | ||
} | ||
|
||
ValidationResult<SecurityToken> readResult = ReadToken(token, callContext); | ||
|
@@ -82,12 +70,10 @@ internal async Task<ValidationResult<ValidatedToken>> ValidateTokenAsync( | |
if (validationResult.IsValid) | ||
return validationResult; // No need to unwrap and re-wrap the result. | ||
|
||
StackFrame validationFailureStackFrame = StackFrames.TokenStringValidationFailed ??= new StackFrame(true); | ||
return validationResult.UnwrapError().AddStackFrame(validationFailureStackFrame); | ||
return validationResult.UnwrapError().AddCurrentStackFrame(); | ||
} | ||
|
||
StackFrame readFailureStackFrame = StackFrames.TokenStringReadFailed ??= new StackFrame(true); | ||
return readResult.UnwrapError().AddStackFrame(readFailureStackFrame); | ||
return readResult.UnwrapError().AddCurrentStackFrame(); | ||
} | ||
|
||
/// <inheritdoc/> | ||
|
@@ -99,28 +85,25 @@ internal async Task<ValidationResult<ValidatedToken>> ValidateTokenAsync( | |
{ | ||
if (token is null) | ||
{ | ||
StackFrame nullTokenStackFrame = StackFrames.TokenNull ??= new StackFrame(true); | ||
return ValidationError.NullParameter( | ||
nameof(token), | ||
nullTokenStackFrame); | ||
ValidationError.GetCurrentStackFrame()); | ||
} | ||
|
||
if (validationParameters is null) | ||
{ | ||
StackFrame nullValidationParametersStackFrame = StackFrames.TokenValidationParametersNull ??= new StackFrame(true); | ||
return ValidationError.NullParameter( | ||
nameof(validationParameters), | ||
nullValidationParametersStackFrame); | ||
ValidationError.GetCurrentStackFrame()); | ||
} | ||
|
||
if (token is not JsonWebToken jsonWebToken) | ||
{ | ||
StackFrame notJwtStackFrame = StackFrames.TokenNotJWT ??= new StackFrame(true); | ||
return new ValidationError( | ||
new MessageDetail(TokenLogMessages.IDX10001, nameof(token), nameof(JsonWebToken)), | ||
ValidationFailureType.InvalidSecurityToken, | ||
typeof(ArgumentException), | ||
notJwtStackFrame); | ||
ValidationError.GetCurrentStackFrame()); | ||
} | ||
|
||
BaseConfiguration? currentConfiguration = | ||
|
@@ -200,8 +183,7 @@ await ValidateJWEAsync(jsonWebToken, validationParameters, lkgConfiguration, cal | |
} | ||
|
||
// If we reach this point, the token validation failed and we should return the error. | ||
StackFrame stackFrame = StackFrames.TokenValidationFailed ??= new StackFrame(true); | ||
return result.UnwrapError().AddStackFrame(stackFrame); | ||
return result.UnwrapError().AddCurrentStackFrame(); | ||
} | ||
|
||
private async ValueTask<ValidationResult<ValidatedToken>> ValidateJWEAsync( | ||
|
@@ -215,15 +197,13 @@ private async ValueTask<ValidationResult<ValidatedToken>> ValidateJWEAsync( | |
jwtToken, validationParameters, configuration, callContext); | ||
if (!decryptionResult.IsValid) | ||
{ | ||
StackFrame decryptionFailureStackFrame = StackFrames.DecryptionFailed ??= new StackFrame(true); | ||
return decryptionResult.UnwrapError().AddStackFrame(decryptionFailureStackFrame); | ||
return decryptionResult.UnwrapError().AddCurrentStackFrame(); | ||
} | ||
|
||
ValidationResult<SecurityToken> readResult = ReadToken(decryptionResult.UnwrapResult(), callContext); | ||
if (!readResult.IsValid) | ||
{ | ||
StackFrame readFailureStackFrame = StackFrames.DecryptedReadFailed ??= new StackFrame(true); | ||
return readResult.UnwrapError().AddStackFrame(readFailureStackFrame); | ||
return readResult.UnwrapError().AddCurrentStackFrame(); | ||
} | ||
|
||
JsonWebToken decryptedToken = (readResult.UnwrapResult() as JsonWebToken)!; | ||
|
@@ -233,8 +213,7 @@ await ValidateJWSAsync(decryptedToken!, validationParameters, configuration, cal | |
|
||
if (!validationResult.IsValid) | ||
{ | ||
StackFrame validationFailureStackFrame = StackFrames.JWEValidationFailed ??= new StackFrame(true); | ||
return validationResult.UnwrapError().AddStackFrame(validationFailureStackFrame); | ||
return validationResult.UnwrapError().AddCurrentStackFrame(); | ||
} | ||
|
||
JsonWebToken jsonWebToken = (validationResult.UnwrapResult().SecurityToken as JsonWebToken)!; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. line 248, can we do better (more precise?) than catching the general Exception class? |
||
|
@@ -289,10 +268,7 @@ private async ValueTask<ValidationResult<ValidatedToken>> ValidateJWSAsync( | |
tokenAudiences, jsonWebToken, validationParameters, callContext); | ||
|
||
if (!audienceValidationResult.IsValid) | ||
{ | ||
StackFrame audienceValidationFailureStackFrame = StackFrames.AudienceValidationFailed ??= new StackFrame(true); | ||
return audienceValidationResult.UnwrapError().AddStackFrame(audienceValidationFailureStackFrame); | ||
} | ||
return audienceValidationResult.UnwrapError().AddCurrentStackFrame(); | ||
} | ||
#pragma warning disable CA1031 // Do not catch general exception types | ||
catch (Exception ex) | ||
|
@@ -362,10 +338,7 @@ private async ValueTask<ValidationResult<ValidatedToken>> ValidateJWSAsync( | |
{ | ||
ValidationResult<SecurityToken> actorReadingResult = ReadToken(jsonWebToken.Actor, callContext); | ||
if (!actorReadingResult.IsValid) | ||
{ | ||
StackFrame actorReadingFailureStackFrame = StackFrames.ActorReadFailed ??= new StackFrame(true); | ||
return actorReadingResult.UnwrapError().AddStackFrame(actorReadingFailureStackFrame); | ||
} | ||
return actorReadingResult.UnwrapError().AddCurrentStackFrame(); | ||
|
||
JsonWebToken actorToken = (actorReadingResult.UnwrapResult() as JsonWebToken)!; | ||
ValidationParameters actorParameters = validationParameters.ActorValidationParameters; | ||
|
@@ -374,10 +347,7 @@ await ValidateJWSAsync(actorToken, actorParameters, configuration, callContext, | |
.ConfigureAwait(false); | ||
|
||
if (!innerActorValidationResult.IsValid) | ||
{ | ||
StackFrame actorValidationFailureStackFrame = StackFrames.ActorValidationFailed ??= new StackFrame(true); | ||
return innerActorValidationResult.UnwrapError().AddStackFrame(actorValidationFailureStackFrame); | ||
} | ||
return innerActorValidationResult.UnwrapError().AddCurrentStackFrame(); | ||
|
||
actorValidationResult = innerActorValidationResult; | ||
} | ||
|
@@ -410,10 +380,7 @@ await ValidateJWSAsync(actorToken, actorParameters, configuration, callContext, | |
ValidationResult<SecurityKey> signatureValidationResult = ValidateSignature( | ||
jsonWebToken, validationParameters, configuration, callContext); | ||
if (!signatureValidationResult.IsValid) | ||
{ | ||
StackFrame signatureValidationFailureStackFrame = StackFrames.SignatureValidationFailed ??= new StackFrame(true); | ||
return signatureValidationResult.UnwrapError().AddStackFrame(signatureValidationFailureStackFrame); | ||
} | ||
return signatureValidationResult.UnwrapError().AddCurrentStackFrame(); | ||
|
||
ValidationResult<ValidatedSigningKeyLifetime> issuerSigningKeyValidationResult; | ||
|
||
|
56 changes: 0 additions & 56 deletions
56
src/Microsoft.IdentityModel.JsonWebTokens/JsonWebTokenHandler.ValidateToken.StackFrames.cs
This file was deleted.
Oops, something went wrong.
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.