-
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 cloud instance name validation #2804
Conversation
… instance validation
src/Microsoft.IdentityModel.Tokens/Exceptions/SecurityTokenInvalidCloudInstanceException.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.IdentityModel.Validators/AadTokenValidationParametersExtension.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.IdentityModel.Validators/AadTokenValidationParametersExtension.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.IdentityModel.Validators/AadTokenValidationParametersExtension.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.IdentityModel.Validators/AadTokenValidationParametersExtension.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.IdentityModel.Validators/AadTokenValidationParametersExtension.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.IdentityModel.Validators/AadTokenValidationParametersExtension.cs
Outdated
Show resolved
Hide resolved
…idSigningKeyException and used Ordinal string comparison
src/Microsoft.IdentityModel.Validators/AadTokenValidationParametersExtension.cs
Outdated
Show resolved
Hide resolved
test/Microsoft.IdentityModel.Validators.Tests/AadTokenValidationParametersExtensionTests.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.IdentityModel.Validators/AadTokenValidationParametersExtension.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.IdentityModel.Protocols.OpenIdConnect/OpenIdProviderMetadataNames.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.IdentityModel.Validators/AadTokenValidationParametersExtension.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.IdentityModel.Validators/AadTokenValidationParametersExtension.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.IdentityModel.Validators/AadTokenValidationParametersExtension.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.IdentityModel.Validators/AadTokenValidationParametersExtension.cs
Outdated
Show resolved
Hide resolved
/// </summary> | ||
[Serializable] | ||
public class SecurityTokenInvalidCloudInstanceNameException : SecurityTokenInvalidSigningKeyException | ||
{ |
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.
Suggest the name: SecurityTokenInvalidCloudInstanceException (drop the 'Name') as there may be other reasons in the future.
@GeoK not sure about the need to inherit from SecurityTokenInvalidSigningKeyException, perhaps SecurityTokenException
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.
Renamed class.
in my opinion SecurityTokenInvalidCloudInstanceNameException is still SecurityTokenInvalidSigningKeyException but more specific
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.
Discussed this with Brent offline - it is ok as-is.
Added optional check to prevent using keys that are shared across multiple clouds.
Resolves #2832