-
Notifications
You must be signed in to change notification settings - Fork 413
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
Add Tests for Lifetime Validation Using New Validation Model For SAML2 #2906
Add Tests for Lifetime Validation Using New Validation Model For SAML2 #2906
Conversation
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.
LGTM
if (value.Value <= NotBefore.Value) | ||
throw LogExceptionMessage(new ArgumentException(FormatInvariant(LogMessages.IDX13514, MarkAsNonPII(value), MarkAsNonPII(NotBefore)))); | ||
} | ||
//This should not be checked here, instead fail during validation of the token. Will remove this code once we release new validation model bug #2905 |
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.
nit: I usually find more clear the single line //
comments as they are easier to know where they begin and end when looking at git history or PRs.
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.
In bug #2905 can you make sure this is linked there?
|
||
Saml2SecurityTokenHandler saml2TokenHandler = new Saml2SecurityTokenHandler(); | ||
|
||
// Validate token using TokenValidationParameters |
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.
// Validate token using TokenValidationParameters | |
// Validate token using legacy TokenValidationParameters |
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.
Add Tests for Lifetime Validation Using New Validation Model For SAML2
Description
Adding tests validating lifetime for new implementation of ValidateTokenAsync for Saml2SecurityTokenHandler
Fixes #2906