Skip to content

update Id web to support authentication handlers other than JwtBearer…#1498

Merged
jennyf19 merged 3 commits into
masterfrom
jennyf/MiseMigrationUpdates
Oct 23, 2021
Merged

update Id web to support authentication handlers other than JwtBearer…#1498
jennyf19 merged 3 commits into
masterfrom
jennyf/MiseMigrationUpdates

Conversation

@jennyf19

Copy link
Copy Markdown
Collaborator

… and provide extension method

replaces #1493

@jennyf19
jennyf19 requested a review from jmprieur October 22, 2021 03:29
@sonarqubecloud

Copy link
Copy Markdown

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

71.0% 71.0% Coverage
0.0% 0.0% Duplication

/// <summary>
/// Web API authentication builder.
/// </summary>
public static class WebApiBuildersInternal

@GeoK GeoK Oct 22, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: a bit odd to suffix a public class with 'Internal'. #Resolved

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

we are trying to hide it. yes, it's odd.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

will go with WebApiBuilders but leave it in the internal namespace

public static class WebApiBuildersInternal
{
/// <summary>
/// Enables token acquisition which is not specific to JWT, such as when using Microsoft.Identity.Service.Essentials (MISE).

@GeoK GeoK Oct 22, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Don't need to call-out MISE specifically. It now allows a higher level abstraction of security token (i.e. System.IdentityModel.Tokens.Jwt and more modern, Microsoft.IdentityModel.JsonWebTokens) to be used with IdWeb. #Resolved

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

that's beautiful! well said!

@GeoK

GeoK commented Oct 22, 2021

Copy link
Copy Markdown
Member

Approving Wilson specific changes.

@jmprieur jmprieur left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM
Thanks @jennyf19

@jennyf19
jennyf19 merged commit 491b8ef into master Oct 23, 2021
@jennyf19
jennyf19 deleted the jennyf/MiseMigrationUpdates branch October 23, 2021 03:45
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