review changes

Author: derisen

3-Authorization-II/1-call-api/API/TodoListAPI/Controllers/TodoListController.cs

@@ -28,6 +28,25 @@ public TodoListController(TodoContext context)
  _context = context;
  }
 
+ /// <summary>
+ /// Indicates if the AT presented has application or delegated permissions.
+ /// </summary>
+ /// <returns></returns>
+ private bool IsAppOnlyToken()
+ {
+ // Add in the optional 'idtyp' claim to check if the access token is coming from an application or user.
+ // See: https://docs.microsoft.com/en-us/azure/active-directory/develop/active-directory-optional-claims
+ if (HttpContext.User.Claims.Any(c => c.Type == "idtyp"))
+ {
+ return HttpContext.User.Claims.Any(c => c.Type == "idtyp" && c.Value == "app");
+ }
+ else
+ {
+ // alternatively, if an AT contains the roles claim but no scp claim, that indicates it's an app token
+ return HttpContext.User.Claims.Any(c => c.Type == "roles") && HttpContext.User.Claims.Any(c => c.Type != "scp");
+ }
+ }
+
  // GET: api/TodoItems
  [HttpGet]
  /// <summary>
@@ -49,7 +68,7 @@ public TodoListController(TodoContext context)
  )]
  public async Task<ActionResult<IEnumerable<TodoItem>>> GetTodoItems()
  {
- if (HasDelegatedPermissions(new string[] { _todoListRead, _todoListReadWrite }))
+ if (!IsAppOnlyToken())
  {
  /// <summary>
  /// The 'oid' (object id) is the only claim that should be used to uniquely identify
@@ -65,12 +84,10 @@ public async Task<ActionResult<IEnumerable<TodoItem>>> GetTodoItems()
  /// </summary>
  return await _context.TodoItems.Where(x => x.Owner == HttpContext.User.GetObjectId()).ToListAsync();
  }
- else if (HasApplicationPermissions(new string[] { _todoListReadAll, _todoListReadWriteAll }))
+ else
  {
  return await _context.TodoItems.ToListAsync();
  }
-
- return null;
  }
 
  // GET: api/TodoItems/5
@@ -83,16 +100,14 @@ public async Task<ActionResult<TodoItem>> GetTodoItem(int id)
  {
  // if it only has delegated permissions, then it will be t.id==id && x.Owner == owner
  // if it has app permissions the it will return t.id==id
- if (HasDelegatedPermissions(new string[] { _todoListRead, _todoListReadWrite }))
+ if (!IsAppOnlyToken())
  {
  return await _context.TodoItems.FirstOrDefaultAsync(t => t.Id == id && t.Owner == HttpContext.User.GetObjectId());
  }
- else if (HasApplicationPermissions(new string[] { _todoListReadAll, _todoListReadWriteAll }))
+ else
  {
  return await _context.TodoItems.FirstOrDefaultAsync(t => t.Id == id);
  }
-
- return null;
  }
 
  // PUT: api/TodoItems/5
@@ -111,31 +126,31 @@ public async Task<IActionResult> PutTodoItem(int id, TodoItem todoItem)
  }
 
 
- if (HasDelegatedPermissions(new string[] { _todoListReadWrite })
- && _context.TodoItems.Any(x => x.Id == id && x.Owner == HttpContext.User.GetObjectId())
- && todoItem.Owner == HttpContext.User.GetObjectId()
- ||
- HasApplicationPermissions(new string[] { _todoListReadWriteAll })
- )
- {
- _context.Entry(todoItem).State = EntityState.Modified;
-
- try
+ if ((!IsAppOnlyToken() && _context.TodoItems.Any(x => x.Id == id && x.Owner == HttpContext.User.GetObjectId()))
+ ||
+ IsAppOnlyToken())
  {
- await _context.SaveChangesAsync();
- }
- catch (DbUpdateConcurrencyException)
- {
- if (!_context.TodoItems.Any(e => e.Id == id))
- {
- return NotFound();
- }
- else
+ if (_context.TodoItems.Any(x => x.Id == id && x.Owner == HttpContext.User.GetObjectId()))
  {
- throw;
+ _context.Entry(todoItem).State = EntityState.Modified;
+
+ try
+ {
+ await _context.SaveChangesAsync();
+ }
+ catch (DbUpdateConcurrencyException)
+ {
+ if (!_context.TodoItems.Any(e => e.Id == id))
+ {
+ return NotFound();
+ }
+ else
+ {
+ throw;
+ }
+ }
  }
  }
- }
 
  return NoContent();
  }
@@ -152,7 +167,7 @@ public async Task<ActionResult<TodoItem>> PostTodoItem(TodoItem todoItem)
  {
  string owner = HttpContext.User.GetObjectId();
 
- if (HasApplicationPermissions(new string[] { _todoListReadWriteAll }))
+ if (IsAppOnlyToken())
  {
  // with such a permission any owner name is accepted
  owner = todoItem.Owner;
@@ -182,39 +197,36 @@ public async Task<ActionResult<TodoItem>> DeleteTodoItem(int id)
  return NotFound();
  }
 
- if ((HasDelegatedPermissions(new string[] { _todoListReadWrite })
- && _context.TodoItems.Any(x => x.Id == id && x.Owner == HttpContext.User.GetObjectId()))
- || HasApplicationPermissions(new string[] { _todoListReadWriteAll }))
+ if ((!IsAppOnlyToken() && _context.TodoItems.Any(x => x.Id == id && x.Owner == HttpContext.User.GetObjectId()))
+ ||
+ IsAppOnlyToken())
  {
  _context.TodoItems.Remove(todoItem);
  await _context.SaveChangesAsync();
- return todoItem;
- }
- else
- {
- return BadRequest();
  }
+
+ return NoContent();
  }
 
- // Checks if the presented token has application permissions
- private bool HasApplicationPermissions(string[] permissionsNames)
- {
- var rolesClaim = User.Claims.Where(
- c => c.Type == ClaimConstants.Roles || c.Type == ClaimConstants.Role)
- .SelectMany(c => c.Value.Split(' '));
+ //// Checks if the presented token has application permissions
+ //private bool HasApplicationPermissions(string[] permissionsNames)
+ //{
+ // var rolesClaim = User.Claims.Where(
+ // c => c.Type == ClaimConstants.Roles || c.Type == ClaimConstants.Role)
+ // .SelectMany(c => c.Value.Split(' '));
 
- var result = rolesClaim.Any(v => permissionsNames.Any(p => p.Equals(v)));
+ // var result = rolesClaim.Any(v => permissionsNames.Any(p => p.Equals(v)));
 
- return result;
- }
+ // return result;
+ //}
 
- // Checks if the presented token has delegated permissions
- private bool HasDelegatedPermissions(string[] scopesNames)
- {
- var result = (User.FindFirst(ClaimConstants.Scp) ?? User.FindFirst(ClaimConstants.Scope))?
- .Value.Split(' ').Any(v => scopesNames.Any(s => s.Equals(v)));
+ //// Checks if the presented token has delegated permissions
+ //private bool HasDelegatedPermissions(string[] scopesNames)
+ //{
+ // var result = (User.FindFirst(ClaimConstants.Scp) ?? User.FindFirst(ClaimConstants.Scope))?
+ // .Value.Split(' ').Any(v => scopesNames.Any(s => s.Equals(v)));
 
- return result ?? false;
- }
+ // return result ?? false;
+ //}
  }
 }

3-Authorization-II/1-call-api/API/TodoListAPI/Startup.cs

@@ -1,4 +1,6 @@
 
+using System.Linq;
+using System.Net;
 using Microsoft.AspNetCore.Builder;
 using Microsoft.AspNetCore.Hosting;
 using Microsoft.Extensions.Hosting;
@@ -7,6 +9,7 @@
 using Microsoft.EntityFrameworkCore;
 using Microsoft.Identity.Web;
 using Microsoft.AspNetCore.Authentication.JwtBearer;
+using Microsoft.IdentityModel.Logging;
 
 using TodoListAPI.Models;
 
@@ -41,21 +44,24 @@ public void ConfigureServices(IServiceCollection services)
  /// Bear in mind that you can do any of the above checks within the individual routes and/or controllers as well.
  /// For more information, visit: https://docs.microsoft.com/azure/active-directory/develop/access-tokens#validate-the-user-has-permission-to-access-this-data
  /// </summary>
- options.Events.OnTokenValidated = async context =>
- {
- // Uncomment the lines below to validate the caller's tenant ID.
+ 
+ //options.Events.OnTokenValidated = async context =>
+ //{
+ // string[] allowedClientApps = { /* list of client ids to allow */ };
 
- // string[] allowedTenants = {/* add a list of tenant IDs */ };
- // string tenantId = context.Principal.Claims.FirstOrDefault(x => x.Type == "tid")?.Value;
+ // string clientappId = context?.Principal?.Claims
+ //  .FirstOrDefault(x => x.Type == "azp" || x.Type == "appid")?.Value;
 
- // if (!allowedTenants.Contains(tenantId))
- // {
- // throw new Exception("This tenant is not authorized");
- // }
-
- };
+ // if (!allowedClientApps.Contains(clientappId))
+ // {
+ // throw new System.Exception("This client is not authorized");
+ // }
+ //};
  }, options => { Configuration.Bind("AzureAd", options); });
 
+ // The following flag can be used to get more descriptive errors in development environments
+ IdentityModelEventSource.ShowPII = false;
+
  services.AddDbContext<TodoContext>(opt => opt.UseInMemoryDatabase("TodoList"));
 
  services.AddControllers();