Updating the sample to ASP.NET Core 2.0 #24
Conversation
Taking: - the TodoListWebApp from active-directory-dotnet-webapp-openidconnect-aspnetcore - the TodoListService from active-directory-dotnet-native-aspnetcore
The way I proceeded is starting from: - the TodoListService of the following sample https://github.com/Azure-Samples/active-directory-dotnet-native-aspnetcore (the code for the TodoListService is identifical) - the open id connect WebApp which is the object of the following sample: https://github.com/Azure-Samples/active-directory-dotnet-webapp-openidconnect-aspnetcore sample This was the object of commit bdd265d - In the TodoListService and the TodoListWebApp upgrading from AspNetCore 2.0.3 to 2.0.5 Then, from the the OpenIdConnect WebApp for ASP.Net Core, I've enabled calling the TodoListService - Updating the AzureAdOptions to compute the Authority from the instance and the tenantID, and adding two other configuration options for the resourceId of the TodoListService (its clientId) and the base address for this service. - Adding a TodoListItem in models - Adding a NaiveSessionCache class in a new Utils folder - Adding a TodoListController and a TodoList view, as well as a "Todo List" entry in the toolbar of the Web API - Update the SignOut() method of the AccountController to clear the cache for the user. - Updating AzureAdAuthenticationBuilderExtensions to request an authorization code, and redeem it so that the token cache contains a token for the user, so that it can be used by the TodoController
|
No issues were found in this pull request. |
|
No issues were found in this pull request. |
|
|
||
| <ItemGroup> | ||
| <PackageReference Include="Microsoft.AspNetCore.All" Version="2.0.3" /> | ||
| <PackageReference Include="Microsoft.AspNetCore.All" Version="2.0.5" /> |
There was a problem hiding this comment.
Updated the NuGet package from ASP.NET Core 2.0.3 to 2.0.5
jmprieur
left a comment
jmprieur
left a comment
There was a problem hiding this comment.
I've annotated the pull request with comments to explain certain interesting aspects.
| // Without overriding the response type (which by default is id_token), the OnAuthorizationCodeReceived event is not called. | ||
| // but instead OnTokenValidated event is called. Here we request both so that OnTokenValidated is called first which | ||
| // ensures that context.Principal has a non-null value when OnAuthorizeationCodeReceived is called | ||
| options.ResponseType = "id_token code"; |
There was a problem hiding this comment.
this bit seems to be a change of behavior in ASP.NET Core 2.0. This also provide an answer to issue #15
| /// <summary> | ||
| /// Instance of the settings for this Web application (to be used in controllers) | ||
| /// </summary> | ||
| public static AzureAdOptions Settings { set; get; } |
There was a problem hiding this comment.
These settings need to be accessed from several locations, including the controller. Since they won't change during the execution of the Web App, I've chosen to provide them as a static member.
|
|
||
| app.UseStaticFiles(); | ||
|
|
||
| app.UseSession(); // Needs to be app.UseAuthentication() and app.UseMvc() otherwise you will get an exception "Session has not been configured for this application or request." |
There was a problem hiding this comment.
The position of UseSession() relatively to UseAuthentication() and UseMvc() is important (it needs to be just before them).
| // Coordinates of the TodoListService | ||
| "TodoListResourceId": "[Enter the Client Id (Application ID) of the TodoListService, obtained from the Azure portal), e.g. 11111111-1111-1111-11111111111111111]", // ClientId of the TodoList Service | ||
| "TodoListBaseAddress": "https://localhost:44351" | ||
| }, |
There was a problem hiding this comment.
The appsettings.json for the ASP.NET application got enriched with:
- the client secret of the TodoList Web App, which is necessary for the confidential client flow to get a token for the TodoListService Web API
- the coordinates of the TodoListService (resourceId and base URL)
jmprieur
requested review from
andretms,
danieldobalian,
dstrockis and
vibronet
|
@jmprieur is README.md the document? I couldn't find another .md file. Are you looking for a technical review, editor review, testing instructions review? |
|
@jmprieur Can you ping the reviewers so we can get this published? |
|
No issues were found in this pull request. |
Updating the sample to ASP.NET Core 2.0
It's probably better to skip commit bdd265d which brings a lot of churn by copying code from already upgraded samples (see below)
The following commits are simpler to understand
Code for the service
The code for the service is exactly the same as in the active-directory-dotnet-native-aspnetcore same. Please refer to the readme of that sample to understand how to it was built
Code for ASP.NET Web App
The code for the ASP.NET Web App is based on the code of the active-directory-dotnet-webapp-openidconnect-aspnetcore sample. Please read the "About The code" section of that sample first.
Then, based on that code, the following modifications were applied:
Authorityfrom theinstanceand thetenantID, and adding two other configuration options forClientSecret, theresourceIdof TodoListService (its clientId) and the base address for this service.TodoListItemin models to deserialize the Json sent by the TodoListServiceNaiveSessionCacheclass in a new Utils folder which serves as a token cache which livetime is the duration of the session. Updated theStartup.csfile accordingly to add sessions.TodoListControllerand aTodoview, as well as a "Todo List" entry in the toolbar of the Web API. This is where most of the interesting code isSignOut()method of theAccountControllerto clear the cache for the user when s/he signs-out.AzureAdAuthenticationBuilderExtensions.csto request an authorization code, and redeem it, getting an access token to the Azure AD graph (https://graph.windows.com), so that the token cache contains a token for the user. This token will be used by theTodoControllerto request another token for the TodoListService