review complete

Author: Kalyan Krishna

2-WebApp-graph-user/2-3-Multi-Tenant/Controllers/HomeController.cs

@@ -23,12 +23,10 @@ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
  */
 
 using Microsoft.AspNetCore.Authorization;
-using Microsoft.AspNetCore.Http.Authentication;
 using Microsoft.AspNetCore.Mvc;
 using Microsoft.Identity.Web;
 using System.Diagnostics;
 using System.Linq;
-using System.Threading.Tasks;
 using WebApp_OpenIDConnect_DotNet.DAL;
 using WebApp_OpenIDConnect_DotNet.Models;
 
@@ -44,32 +42,41 @@ public HomeController(SampleDbContext dbContext)
             this.dbContext = dbContext;
         }
 
+        /// <summary>
+        /// Retrieves a list all authorized tenants to be displayed in a table, for demonstration purpose only
+        /// </summary>
+        /// <returns></returns>
         public IActionResult Index()
         {
-            //Getting all authorized tenants to be displayed on a table, for demonstration purpose
             var authorizedTenants = dbContext.AuthorizedTenants.Where(x => x.TenantId != null && x.AuthorizedOn != null).ToList();
             return View(authorizedTenants);
         }
 
+        /// <summary>Deletes the selected tenant from the app's own DB (off-boarding).</summary>
+        /// <param name="id">The tenant Id.</param>
+        /// <returns></returns>
         public IActionResult DeleteTenant(string id)
         {
             var tenants = dbContext.AuthorizedTenants.Where(x => x.TenantId == id).ToList();
             dbContext.RemoveRange(tenants);
             dbContext.SaveChanges();
 
-            var signedUserTenant = User.GetTenantId();
+            var signedUsersTenant = User.GetTenantId();
 
-            // If the user deletes its own tenant from the list, they should be signed-out
-            if (id == signedUserTenant)
+            // If the user deletes its own tenant from the list, they would also be signed-out
+            if (id == signedUsersTenant)
                 return RedirectToAction("SignOut", "Account", new { area = "AzureAD" });
             else
                 return RedirectToAction("Index");
         }
 
+        /// <summary>
+        /// If you landed here, its because you tried to sign-in with a user account from a tenant that hasn't onboarded the application yet.
+        /// </summary>
+        /// <returns></returns>
         [ResponseCache(Duration = 0, Location = ResponseCacheLocation.None, NoStore = true)]
         public IActionResult UnauthorizedTenant()
         {
-            //If you landed here, is because you tried to sign-in with a user from a tenant that hasnt been onboarded in the application yet.
             return View();
         }
 

2-WebApp-graph-user/2-3-Multi-Tenant/Controllers/OnboardingController.cs

@@ -54,6 +54,8 @@ public IActionResult SignUp()
             return View();
         }
 
+        /// <summary>This action builds the admin consent Url to let the tenant admin consent and provision a service principal of their app in their tenant.</summary>
+        /// <returns></returns>
         [HttpPost]
         [ValidateAntiForgeryToken]
         public IActionResult Onboard()
@@ -64,7 +66,7 @@ public IActionResult Onboard()
             AuthorizedTenant authorizedTenant = new AuthorizedTenant
             {
                 CreatedOn = DateTime.Now,
-                TempAuthorizationCode = stateMarker //Use the stateMarker as a tempCode, so we can find this entity on the ProcessCode method
+                TempAuthorizationCode = stateMarker //Use the stateMarker as a tempCode, so we can locate this entity in the ProcessCode method
             };
 
             // Saving a temporary tenant to validate the stateMarker on the admin consent response
@@ -76,41 +78,57 @@ public IActionResult Onboard()
                 this.Request.Host,
                 this.Request.PathBase);
 
-            // Create an OAuth2 request, using the web app as the client.This will trigger a consent flow that will provision the app in the target tenant.
+            // Create an OAuth2 request, using the web app as the client. This will trigger a consent flow that will provision the app in the target tenant.
+            // Refer to https://docs.microsoft.com/azure/active-directory/develop/v2-admin-consent for details about the Url format being constructed below
             string authorizationRequest = string.Format(
                 "{0}common/v2.0/adminconsent?client_id={1}&redirect_uri={2}&state={3}&scope={4}",
                 azureADOptions.Instance,
-                Uri.EscapeDataString(azureADOptions.ClientId), // The application Id on Azure Portal
-                Uri.EscapeDataString(currentUri + "Onboarding/ProcessCode"), //Uri that will get redirected after the admin has consented
-                Uri.EscapeDataString(stateMarker), // The state parameter is used to validate the response, preventing a man-in-the-middle attack, and it will also be used to identify the entity on ProcessCode action.
-                Uri.EscapeDataString("https://graph.microsoft.com/.default")); // The scopes to be presented to the admin. Here we are using the static scope /.default. 
+                Uri.EscapeDataString(azureADOptions.ClientId),                  // The application Id as obtained from the Azure Portal
+                Uri.EscapeDataString(currentUri + "Onboarding/ProcessCode"),    // Uri that the admin will be redirected to after the consent
+                Uri.EscapeDataString(stateMarker),                              // The state parameter is used to validate the response, preventing a man-in-the-middle attack, and it will also be used to identify this request in the ProcessCode action.
+                Uri.EscapeDataString("https://graph.microsoft.com/.default"));  // The scopes to be presented to the admin to consent. Here we are using the static scope '/.default' (https://docs.microsoft.com/azure/active-directory/develop/v2-permissions-and-consent#the-default-scope).
 
             return Redirect(authorizationRequest);
         }
 
-        // This is the redirect Uri for the admin consent authorization
-        public async Task<IActionResult> ProcessCode(string tenant, string error, string error_description, string resource, string state)
+        /// <summary>
+        /// This handler is used to process the response after the admin consent process is complete.
+        /// </summary>
+        /// <param name="tenant">The directory tenant that granted your application the permissions it requested, in GUID format..</param>
+        /// <param name="error">An error code string that can be used to classify types of errors that occur, and can be used to react to errors..</param>
+        /// <param name="error_description">A specific error message that can help a developer identify the root cause of an error..</param>
+        /// <param name="admin_consent">Will be set to True to indicate that this response occurred on an admin consent flow..</param>
+        /// <param name="state">A value included in the request that also will be returned in the token response. It can be a string of any content you want. The state is used to encode information about the user's state in the app before the authentication request occurred, such as the page or view they were on..</param>
+        /// <remarks>Refer to https://docs.microsoft.com/en-us/azure/active-directory/develop/v2-admin-consent for details on the response</remarks>
+        /// <returns></returns>
+        public async Task<IActionResult> ProcessCode(string tenant, string error, string error_description, string admin_consent, string state)
         {
             if (error != null)
             {
                 TempData["ErrorMessage"] = error_description;
                 return RedirectToAction("Error", "Home");
             }
 
+            if (admin_consent.ToUpper() != "TRUE")
+            {
+                TempData["ErrorMessage"] = "The admin consent operation failed.";
+                return RedirectToAction("Error", "Home");
+            }
+
             var authenticationProperties = new AuthenticationProperties { RedirectUri = "Home/Index" };
 
             // If tenant is already authorized, there is no need updated its record
             if (dbContext.AuthorizedTenants.FirstOrDefault(x => x.TenantId == tenant) != null)
             {
-                // Challenge an authentication so dotnet can set the user identity claims. 
+                // Create a Sign-in challenge to re-authenticate the user again as we need claims from the user's id_token.
                 // Since the user will have a session on AAD already, they wont need to select an account again.
                 return Challenge(authenticationProperties, AzureADDefaults.OpenIdScheme);
             }
 
-            // Find a tenant carrying a TempAuthorizationCode that we previously saved
+            // Find a tenant record matching the TempAuthorizationCode that we previously saved in the Onboard()
             var preAuthorizedTenant = dbContext.AuthorizedTenants.FirstOrDefault(a => a.TempAuthorizationCode == state);
 
-            // If we don't find it, return an error because the state param was not generated from this app
+            // If we don't find it, return an error because the state param was not generated from this app and we do not wish to process this request
             if (preAuthorizedTenant == null)
             {
                 TempData["ErrorMessage"] = "State verification failed.";
@@ -124,8 +142,8 @@ public async Task<IActionResult> ProcessCode(string tenant, string error, string
 
                 await dbContext.SaveChangesAsync();
 
-                // Challenge an authentication so dotnet can set the user identity claims. 
-                // Since the user will have a session on AAD already, they wont need to select an account again.
+                // Create a Sign-in challenge to re-authenticate the user again as we need claims from the user's id_token.
+                // Since the user will have a session on AAD already, they wont need to select an account again
                 return Challenge(authenticationProperties, AzureADDefaults.OpenIdScheme);
             }
         }

2-WebApp-graph-user/2-3-Multi-Tenant/Controllers/TodoListController.cs

@@ -22,16 +22,14 @@ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 SOFTWARE.
  */
 
-using System;
-using System.Collections.Generic;
-using System.Linq;
-using System.Threading.Tasks;
 using Microsoft.AspNetCore.Authorization;
 using Microsoft.AspNetCore.Mvc;
 using Microsoft.AspNetCore.Mvc.Rendering;
 using Microsoft.Identity.Web;
-using WebApp_OpenIDConnect_DotNet.Services;
+using System.Linq;
+using System.Threading.Tasks;
 using WebApp_OpenIDConnect_DotNet.Models;
+using WebApp_OpenIDConnect_DotNet.Services;
 using WebApp_OpenIDConnect_DotNet.Utils;
 
 namespace WebApp_OpenIDConnect_DotNet.Controllers
@@ -90,15 +88,15 @@ public async Task<IActionResult> Edit(int id)
         {
             TodoItem todoItem = await _todoItemService.Get(id, User);
 
-            if(todoItem == null)
+            if (todoItem == null)
             {
                 TempData["ErrorMessage"] = "Item not found";
                 return RedirectToAction("Error", "Home");
             }
 
             var userTenant = User.GetTenantId();
 
-            // Acquiring token for graph using the user's tenantId, so it can return all the users from their tenant
+            // Acquiring token for graph in the signed-in users tenant, so it can be used to retrieve all the users from their tenant
             var graphAccessToken = await _tokenAcquisition.GetAccessTokenOnBehalfOfUserAsync(new string[] { GraphScope.UserReadAll }, userTenant);
 
             TempData["UsersDropDown"] = (await _msGraphService.GetUsersAsync(graphAccessToken))

2-WebApp-graph-user/2-3-Multi-Tenant/README.md

@@ -178,7 +178,7 @@ If you try to sign-in with a tenant that hasn't been "onboarded" yet, you will l
 
 ![Unauthorized Tenant](ReadmeFiles/unauthorized-tenant.png)
 
-> :warning: If you onboarded your tenant using this sample in the past already and getting the **AADSTS650051** error when onboarding again, please refer to the [Error AADSTS650051](#error-aadsts650051) section below.
+> :warning: If you had onboarded your tenant using this sample in the past and now getting the **AADSTS650051** error when onboarding again, please refer to the [Error AADSTS650051](#error-aadsts650051) section below to mitigate this error.
 
 #### ToDo List
 
@@ -215,9 +215,9 @@ services.AddAuthentication(AzureADDefaults.AuthenticationScheme)
 
  You can read about the various endpoints of the Microsoft Identity Platform [here](https://docs.microsoft.com/azure/active-directory/develop/active-directory-v2-protocols#endpoints).
 
-### Service principle provision for new tenants (onboarding process)
+### Service principle provisioning for new tenants (onboarding process)
 
-For a multi-tenant app, its service principle will be created on all the users' tenants that have signed-in at least once. Some might want that only tenant admins accept the service principle provisioning. For that, we are using the [admin consent endpoint](https://docs.microsoft.com/azure/active-directory/develop/v2-admin-consent) for the onboarding process on the `OnboardingController.cs`. The `Onboard` action and corresponding view, simulate an onboarding experience.
+For a multi-tenant app to work across tenants, its service principle will need to be provisioned in the users' tenant. It can either happen when the first user signs in, or most tenant admins only allow a tenant admin to carry out the service principle provisioning. For provisioning, we will be using the [admin consent endpoint](https://docs.microsoft.com/azure/active-directory/develop/v2-admin-consent) for the onboarding process. The code for this is provided in the `OnboardingController.cs`. The `Onboard` action and corresponding view, simulate the onboarding flow and experience.
 
 ```csharp
 [HttpPost]
@@ -236,13 +236,13 @@ public IActionResult Onboard()
 }
 ```
 
-This results in an OAuth2 code grant request that triggers the admin consent flow and creates the service principle in the admin's tenant. The `state` parameter is used to validate the response, preventing a man-in-the-middle attack. Then, the `ProcessCode` action receives the authorization code from Azure AD and, if they appear valid, it creates an entry in the application database for the new customer.
+This results in an OAuth2 code grant request that triggers the admin consent flow and creates the service principle in the admin's tenant. The `state` parameter is used to validate the response, preventing a man-in-the-middle attack. Then, the `ProcessCode` action receives the authorization code from Azure AD and, if they appear valid, we create an entry in the application database for the new customer.
 
-The `https://graph.microsoft.com/.default` is a static scope. You can find more about static scope on [this link.](https://docs.microsoft.com/azure/active-directory/develop/v2-admin-consent#request-the-permissions-from-a-directory-admin)
+The `https://graph.microsoft.com/.default` is a static scope that allows the tenant admin to consent for all permissions in one go. You can find more about static scope on [this link.](https://docs.microsoft.com/azure/active-directory/develop/v2-admin-consent#request-the-permissions-from-a-directory-admin)
 
 ### Custom token validation allowing only registered tenants
 
-On the `Startup.cs` we are calling `AddMicrosoftIdentityPlatformAuthentication` to configure the authentication, and it also validates that the token issuer is from AAD.
+On the `Startup.cs` we are calling `AddMicrosoftIdentityPlatformAuthentication` to configure the authentication, and within that method, we validates that the token issuer is from AAD.
 
 ```csharp
 options.TokenValidationParameters.IssuerValidator = AadIssuerValidator.GetIssuerValidator(options.Authority).Validate;
@@ -283,17 +283,17 @@ services.Configure<OpenIdConnectOptions>(AzureADDefaults.OpenIdScheme, options =
 
 ### Data partitioning by tenant
 
-There are two common scenarios regarding data partition on a multi-tenant app. Having a separate database for each tenant or having a single database and using the **tenantId** to distinguish the data from each tenant. In this sample, we used the single database approach to save the todo items.
+There are two common scenarios regarding data partition on a multi-tenant app. Having a separate database for each tenant or having a single database and using the **tenantId** to separate the data of each tenant. In this sample, we have taken the single database approach to save the ToDo items for all users from all tenants.
 
-`TodoListController.cs` have basic CRUD actions for `todoItem` and each operation takes into account the signed user's **tenantId** to separate data from each tenant. The tenantId can be found in the user' claims.
+`TodoListController.cs` has the basic CRUD actions for `ToDoItem` and each operation takes into account the signed user's **tenantId** to separate data from each tenant. The tenantId can be found in the user' claims.
 
-### Microsoft Graph token by tenant
+### Acquiring Access token for Microsoft Graph for each tenant
 
-If a multi-tenant app needs to acquire a token from Graph to read data from the signed user's tenant, the token must be issued with their tenantId authority and not where the application is registered. This feature is being showed on the **Edit** action result on `todoListController.cs`.
+If a multi-tenant app needs to acquire an access token for Microsoft Graph to be able to read data from the signed user's tenant, the token must be issued from their tenanted authority and not from the tenant where the SaaS application is registered. This feature is being showed on the **Edit** action result on `todoListController.cs`.
 
 ```csharp
 var userTenant = User.GetTenantId();
-// Acquiring token for graph using the user's tenantId, so it can return all the users from their tenant
+// Acquiring token for graph using the user's tenant, so it can return all the users from their tenant
 var graphAccessToken = await _tokenAcquisition.GetAccessTokenOnBehalfOfUserAsync(new string[] { GraphScope.UserReadAll }, userTenant);
 ```
 

2-WebApp-graph-user/2-3-Multi-Tenant/Services/IMSGraphService.cs

@@ -32,4 +32,4 @@ public interface IMSGraphService
     {
         Task<IEnumerable<User>> GetUsersAsync(string accessToken);
     }
-}
+}

2-WebApp-graph-user/2-3-Multi-Tenant/Services/MSGraphService.cs

@@ -22,17 +22,17 @@ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 SOFTWARE.
  */
 
-using Microsoft.AspNetCore.Mvc.Rendering;
 using Microsoft.Graph;
 using System;
 using System.Collections.Generic;
 using System.Diagnostics;
-using System.Linq;
 using System.Net.Http.Headers;
 using System.Threading.Tasks;
 
 namespace WebApp_OpenIDConnect_DotNet.Services
 {
+    /// <summary>Provides helper methods built over MS Graph SDK</summary>
+    /// <seealso cref="WebApp_OpenIDConnect_DotNet.Services.IMSGraphService" />
     public class MSGraphService : IMSGraphService
     {
         // the Graph SDK's GraphServiceClient
@@ -58,7 +58,7 @@ public async Task<IEnumerable<User>> GetUsersAsync(string accessToken)
                     .Filter($"accountEnabled eq true")
                     .Select("id, userPrincipalName")
                     .GetAsync();
-                
+
                 if (users?.CurrentPage.Count > 0)
                 {
                     return users;
@@ -81,6 +81,14 @@ private void PrepareAuthenticatedClient(string accessToken)
         {
             try
             {
+                /***
+                <!--Microsoft Azure AD Graph API endpoint,
+                'https://graph.microsoft.com'   Microsoft Graph global service
+                'https://graph.microsoft.us' Microsoft Graph for US Government
+                'https://graph.microsoft.de' Microsoft Graph Germany
+                'https://microsoftgraph.chinacloudapi.cn' Microsoft Graph China
+                -->
+                 * ***/
                 graphServiceClient = new GraphServiceClient("https://graph.microsoft.com/beta",
                                                                      new DelegateAuthenticationProvider(
                                                                          async (requestMessage) =>
@@ -97,4 +105,4 @@ await Task.Run(() =>
             }
         }
     }
-}
+}