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

Misc updates for the JWT sample application #1009

Merged
merged 7 commits into from
Apr 6, 2023
Merged

Conversation

Shane32
Copy link
Member

@Shane32 Shane32 commented Mar 20, 2023

  • Pull TokenValidationParameters from the call to AddJwtBearer so that the reference to the static property JwtHelper.Instance is not necessary
  • Eliminate the static property JwtHelper.Instance
  • Reconfigure Program.cs to use a local variable instead of a static property
  • Reconfigure OAuthController to pull JwtHelper from DI
  • Add the 'iat' claim to generated tokens (required for Open ID Connect)
  • Bump Microsoft.AspNetCore.Authentication.JwtBearer since the project is a .NET 7 sample

@Shane32 Shane32 self-assigned this Mar 20, 2023
@codecov-commenter
Copy link

Codecov Report

Merging #1009 (8e600f8) into master (9ebcb06) will not change coverage.
The diff coverage is n/a.

❗ Current head 8e600f8 differs from pull request most recent head a58c8c8. Consider uploading reports for the commit a58c8c8 to get more accurate results

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@           Coverage Diff           @@
##           master    #1009   +/-   ##
=======================================
  Coverage   93.42%   93.42%           
=======================================
  Files          44       44           
  Lines        2175     2175           
  Branches      366      366           
=======================================
  Hits         2032     2032           
  Misses        102      102           
  Partials       41       41           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@Shane32
Copy link
Member Author

Shane32 commented Mar 20, 2023

As a side note, I would prefer to add another nuget package for JWT handling to this repository, specifically for the IWebSocketAuthenticationService implementation (which does not require JwtHelper to be useful). I cannot put it in the main package because it requires another dependency. However, this makes an assumption about the format of the websocket initialization packet payload, which may be different from user to user as there is no official standard. It also only works for JWT, and not any other authentication mechanism. So as of now I think we should just leave it as a sample application. I plan to keep this sample code up to date as best I can so users may copy it if they wish into their own application.

@Shane32 Shane32 changed the title Update for the JWT sample Misc updates for the JWT sample application Mar 20, 2023
@Shane32 Shane32 requested a review from sungam3r March 20, 2023 22:50
// this mapping is not performed by Microsoft.IdentityModel.JsonWebTokens.JsonWebTokenHandler
var handler = new System.IdentityModel.Tokens.Jwt.JwtSecurityTokenHandler();
var tokenValidationParameters = _jwtBearerOptionsMonitor.Get(JwtBearerDefaults.AuthenticationScheme).TokenValidationParameters;
var principal = handler.ValidateToken(token, tokenValidationParameters, out var securityToken);
Copy link
Member

Choose a reason for hiding this comment

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

No TokenValidationResult anymore? Does ValidateToken throw in case of any error?

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct; it throws in the case that the token is invalid for any reason (according to the parameters defined in tokenValidationParameters). So for example, if TokenValidationParameters.ValidateIssuer is true, then the issuer must match or an exception is thrown.

@Shane32 Shane32 merged commit 0f97391 into master Apr 6, 2023
@Shane32 Shane32 deleted the update_jwt_sample branch April 6, 2023 15:43
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.

3 participants