Jmprieur/sync from web api tutorial #186
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
in TokenAcquisition.cs from the code which is currently in the ASP.NET Core Web API tutorial
jmprieur
commented
Sep 12, 2019
| /// This method appends in the HttpResponse being sent back to the client, a 403 (forbidden) status code and populates the 'WWW-Authenticate' header with additional information. | ||
| /// The client, when it receives the 403 code with this header, can use the additional information provided in the header to trigger an interaction with the user where the user | ||
| /// can then consent to additional scopes. | ||
| /// Used in Web APIs (which therefore cannot have an interaction with the user). |
There was a problem hiding this comment.
Better comments (from Jenny)
jmprieur
commented
Sep 12, 2019
| /// <param name="scopes">The additional scopes that the user needs to consent to</param> | ||
| /// <param name="msalServiceException"><see cref="MsalUiRequiredException"/> The MsalUiRequiredException that is examined to see if there is case for this response.</param> | ||
|
|
||
| /// <param name="scopes">Scopes to consent to</param> |
There was a problem hiding this comment.
Better comments (from Jenny)
jmprieur
commented
Sep 12, 2019
| /// <param name="msalServiceException"><see cref="MsalUiRequiredException"/> The MsalUiRequiredException that is examined to see if there is case for this response.</param> | ||
|
|
||
| /// <param name="scopes">Scopes to consent to</param> | ||
| /// <param name="msalServiceException"><see cref="MsalUiRequiredException"/> triggering the challenge</param> |
There was a problem hiding this comment.
Better comments (from Jenny)
jmprieur
commented
Sep 12, 2019
| public void ReplyForbiddenWithWwwAuthenticateHeader(IEnumerable<string> scopes, MsalUiRequiredException msalServiceException) | ||
| { | ||
| // A user interaction is required, but we are in a Web API, and therefore, we need to report back to the client through an wwww-Authenticate header https://tools.ietf.org/html/rfc6750#section-3.1 | ||
| string proposedAction = "consent"; | ||
| if (msalServiceException.ErrorCode == MsalError.InvalidGrantError) | ||
| { | ||
| if (AcceptedTokenVersionIsNotTheSameAsTokenVersion(msalServiceException)) | ||
| if (AcceptedTokenVersionMismatch(msalServiceException)) |
There was a problem hiding this comment.
Better method name (from Jenny)
jmprieur
commented
Sep 12, 2019
| { | ||
| throw msalServiceException; | ||
| } | ||
| } | ||
|
|
||
| string consentUrl = $"{application.Authority}/oauth2/v2.0/authorize?client_id={_azureAdOptions.ClientId}" |
There was a problem hiding this comment.
Consent URL to provision the Web API in multi-tenant / MSA
jmprieur
commented
Sep 12, 2019
| @@ -38,20 +38,18 @@ public static class WebApiServiceCollectionExtensions | |||
| this IServiceCollection services, | |||
| IConfiguration configuration, | |||
| X509Certificate2 tokenDecryptionCertificate = null, | |||
| string configSectionName = "AzureAD", | |||
| string configSectionName = "AzureAd", | |||
There was a problem hiding this comment.
Fixing the name of the section (like Kalyan did for the Web App)
jmprieur
commented
Sep 12, 2019
| bool subscribeToJwtBearerMiddlewareDiagnosticsEvents = false) | ||
| { | ||
| services.AddAuthentication(AzureADDefaults.JwtBearerAuthenticationScheme) | ||
| .AddAzureADBearer(options => configuration.Bind(configSectionName, options)); | ||
| services.Configure<AzureADOptions>(options => configuration.Bind(configSectionName, options)); |
There was a problem hiding this comment.
Fixing the binding
|
@TiagoBrenck @kalyankrishna1 |
TiagoBrenck
approved these changes
Sep 19, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Purpose
Does this introduce a breaking change?
Pull Request Type
What kind of change does this Pull Request introduce?
How to Test
What to Check
Verify that the following are valid
Other Information