Skip to content
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

Throws SecurityTokenMalformedTokenException on malformed tokens #2080

Merged
merged 4 commits into from
May 31, 2023

Conversation

dannybtsai
Copy link
Contributor

Currently an ArgumentException is thrown when a token handler fails to parse/read a raw token. Since the ArgumentException is a general exception for any invalid arguments, sometimes it does not provide specific types of errors, ex, invalid header or payload. This PR adds a new exception, SecurityTokenMalformedTokenException, that can be thrown when a malformed token is detected. For example, the exception will be thrown if a SAML token is handled by JsonWebTokenHandler.

This should provide a clearer exception so users to handle it differently.

@victorm-hernandez
Copy link
Contributor

victorm-hernandez commented May 17, 2023

    /// <exception cref="ArgumentException">'jwtEncodedString' contains only whitespace.</exception>

Let's update the XML docs to reflect we are throwing a different exception type.


In reply to: 1551947809


In reply to: 1551947809


Refers to: src/System.IdentityModel.Tokens.Jwt/JwtSecurityToken.cs:24 in 1f748de. [](commit_id = 1f748de, deletion_comment = False)

@victorm-hernandez
Copy link
Contributor

victorm-hernandez commented May 17, 2023

    /// <exception cref="ArgumentException"><paramref name="token"/>.Length is greater than <see cref="TokenHandler.MaximumTokenSizeInBytes"/>.</exception>

ditto: lets update XML docs for the new exception type


In reply to: 1551949110


In reply to: 1551949110


Refers to: src/System.IdentityModel.Tokens.Jwt/JwtSecurityTokenHandler.cs:815 in 1f748de. [](commit_id = 1f748de, deletion_comment = False)

@victorm-hernandez
Copy link
Contributor

victorm-hernandez commented May 17, 2023

    public override ClaimsPrincipal ValidateToken(string token, TokenValidationParameters validationParameters, out SecurityToken validatedToken)

Let's update also the JsonWebTokenHandler and Saml2SecurityTokenHandler ValidateToken methods to be consistent.


In reply to: 1551955852


Refers to: src/System.IdentityModel.Tokens.Jwt/JwtSecurityTokenHandler.cs:835 in 1f748de. [](commit_id = 1f748de, deletion_comment = False)

Copy link
Contributor

@victorm-hernandez victorm-hernandez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🕐

…enException to SecurityTokenMalformedException and updated comments
…eTokenAsync and ValidateTokenAsync.ReadToken when failing to parse/read a token as a valid JWT

Added more comments
@victorm-hernandez
Copy link
Contributor

    public override ClaimsPrincipal ValidateToken(string token, TokenValidationParameters validationParameters, out SecurityToken validatedToken)

Danny created a work item for this
https://identitydivision.visualstudio.com/Engineering/_workitems/edit/2589877


In reply to: 1551955852


Refers to: src/System.IdentityModel.Tokens.Jwt/JwtSecurityTokenHandler.cs:835 in 1f748de. [](commit_id = 1f748de, deletion_comment = False)

Copy link
Contributor

@victorm-hernandez victorm-hernandez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@dannybtsai dannybtsai merged commit 764f88d into dev May 31, 2023
@brentschmaltz brentschmaltz deleted the danny/SecurityTokenMalformedTokenException-dev branch June 2, 2023 21:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants