Skip to content

fix some comments and namings #244

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 8, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Microsoft.Identity.Web/AccountExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ public static ClaimsPrincipal ToClaimsPrincipal(this IAccount account)
return new ClaimsPrincipal(
new ClaimsIdentity(new Claim[]
{
new Claim(ClaimConstants.Oid, account.HomeAccountId.ObjectId),
new Claim(ClaimConstants.Tid, account.HomeAccountId.TenantId),
new Claim(ClaimConstants.Oid, account.HomeAccountId?.ObjectId),
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we even add the claims if the HomeAccountId is null?

new Claim(ClaimConstants.Tid, account.HomeAccountId?.TenantId),
new Claim(ClaimTypes.Upn, account.Username)
})
);
Expand Down
26 changes: 14 additions & 12 deletions Microsoft.Identity.Web/AuthorizeForScopesAttribute.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,8 @@

namespace Microsoft.Identity.Web
{
// TODO: rename to EnsureScopesAttribute ? or MsalAuthorizeForScopesAttribute or AuthorizeForScopesAttribute

/// <summary>
/// Filter used on a controller action to trigger an incremental consent.
/// Filter used on a controller action to trigger incremental consent.
/// </summary>
/// <example>
/// The following controller action will trigger
Expand All @@ -42,7 +40,7 @@ public class AuthorizeForScopesAttribute : ExceptionFilterAttribute
public string ScopeKeySection { get; set; }

/// <summary>
/// Handles the MsaUiRequiredExeception
/// Handles the MsalUiRequiredException
/// </summary>
/// <param name="context">Context provided by ASP.NET Core</param>
public override void OnException(ExceptionContext context)
Expand All @@ -55,7 +53,7 @@ public override void OnException(ExceptionContext context)

if (msalUiRequiredException != null)
{
if (CanBeSolvedByReSignInUser(msalUiRequiredException))
if (CanBeSolvedByReSignInOfUser(msalUiRequiredException))
{
// the users cannot provide both scopes and ScopeKeySection at the same time
if (!string.IsNullOrWhiteSpace(ScopeKeySection) && Scopes != null && Scopes.Length > 0)
Expand Down Expand Up @@ -85,7 +83,7 @@ public override void OnException(ExceptionContext context)
base.OnException(context);
}

private bool CanBeSolvedByReSignInUser(MsalUiRequiredException ex)
private bool CanBeSolvedByReSignInOfUser(MsalUiRequiredException ex)
{
// ex.ErrorCode != MsalUiRequiredException.UserNullError indicates a cache problem.
// When calling an [Authenticate]-decorated controller we expect an authenticated
Expand All @@ -97,22 +95,26 @@ private bool CanBeSolvedByReSignInUser(MsalUiRequiredException ex)
}

/// <summary>
/// Build Authentication properties needed for an incremental consent.
/// Build Authentication properties needed for incremental consent.
/// </summary>
/// <param name="scopes">Scopes to request</param>
/// <param name="ex">MsalUiRequiredException instance</param>
/// <param name="context">current http context in the pipeline</param>
/// <returns>AuthenticationProperties</returns>
private AuthenticationProperties BuildAuthenticationPropertiesForIncrementalConsent(
string[] scopes, MsalUiRequiredException ex, HttpContext context)
string[] scopes,
MsalUiRequiredException ex,
HttpContext context)
{
var properties = new AuthenticationProperties();

// Set the scopes, including the scopes that ADAL.NET / MASL.NET need for the Token cache
string[] additionalBuildInScopes =
{OidcConstants.ScopeOpenId, OidcConstants.ScopeOfflineAccess, OidcConstants.ScopeProfile};
// Set the scopes, including the scopes that ADAL.NET / MSAL.NET need for the token cache
string[] additionalBuiltInScopes =
{OidcConstants.ScopeOpenId,
OidcConstants.ScopeOfflineAccess,
OidcConstants.ScopeProfile};
properties.SetParameter<ICollection<string>>(OpenIdConnectParameterNames.Scope,
scopes.Union(additionalBuildInScopes).ToList());
scopes.Union(additionalBuiltInScopes).ToList());

// Attempts to set the login_hint to avoid the logged-in user to be presented with an account selection dialog
var loginHint = context.User.GetLoginHint();
Expand Down
28 changes: 15 additions & 13 deletions Microsoft.Identity.Web/ClaimsPrincipalExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public static string GetMsalAccountId(this ClaimsPrincipal claimsPrincipal)
/// <summary>
/// Gets the unique object ID associated with the <see cref="ClaimsPrincipal"/>
/// </summary>
/// <param name="claimsPrincipal">the <see cref="ClaimsPrincipal"/> from which to retrieve the unique object id</param>
/// <param name="claimsPrincipal">the <see cref="ClaimsPrincipal"/> from which to retrieve the unique object ID</param>
/// <remarks>This method returns the object ID both in case the developer has enabled or not claims mapping</remarks>
/// <returns>Unique object ID of the identity, or <c>null</c> if it cannot be found</returns>
public static string GetObjectId(this ClaimsPrincipal claimsPrincipal)
Expand All @@ -50,9 +50,9 @@ public static string GetObjectId(this ClaimsPrincipal claimsPrincipal)
/// <summary>
/// Gets the Tenant ID associated with the <see cref="ClaimsPrincipal"/>
/// </summary>
/// <param name="claimsPrincipal">the <see cref="ClaimsPrincipal"/> from which to retrieve the tenant id</param>
/// <param name="claimsPrincipal">the <see cref="ClaimsPrincipal"/> from which to retrieve the tenant ID</param>
/// <returns>Tenant ID of the identity, or <c>null</c> if it cannot be found</returns>
/// <remarks>This method returns the object ID both in case the developer has enabled or not claims mapping</remarks>
/// <remarks>This method returns the tenant ID both in case the developer has enabled or not claims mapping</remarks>
public static string GetTenantId(this ClaimsPrincipal claimsPrincipal)
{
string tenantId = claimsPrincipal.FindFirstValue(ClaimConstants.Tid);
Expand Down Expand Up @@ -95,27 +95,29 @@ public static string GetDomainHint(this ClaimsPrincipal claimsPrincipal)
/// Get the display name for the signed-in user, from the <see cref="ClaimsPrincipal"/>
/// </summary>
/// <param name="claimsPrincipal">Claims about the user/account</param>
/// <returns>A string containing the display name for the user, as brought by Azure AD (v1.0) and Microsoft identity platform (v2.0) tokens,
/// <returns>A string containing the display name for the user, as determined by Azure AD (v1.0) and Microsoft identity platform (v2.0) tokens,
/// or <c>null</c> if the claims cannot be found</returns>
/// <remarks>See https://docs.microsoft.com/azure/active-directory/develop/id-tokens#payload-claims </remarks>
public static string GetDisplayName(this ClaimsPrincipal claimsPrincipal)
{
// Use the claims in an Microsoft identity platform token first
// Use the claims in a Microsoft identity platform token first
string displayName = claimsPrincipal.FindFirstValue(ClaimConstants.PreferredUserName);

// Otherwise fall back to the claims in an Azure AD v1.0 token
if (string.IsNullOrWhiteSpace(displayName))
if (!string.IsNullOrWhiteSpace(displayName))
{
displayName = claimsPrincipal.FindFirstValue(ClaimsIdentity.DefaultNameClaimType);
return displayName;
}

// Finally falling back to name
if (string.IsNullOrWhiteSpace(displayName))
// Otherwise fall back to the claims in an Azure AD v1.0 token
displayName = claimsPrincipal.FindFirstValue(ClaimsIdentity.DefaultNameClaimType);

if (!string.IsNullOrWhiteSpace(displayName))
{
displayName = claimsPrincipal.FindFirstValue(ClaimConstants.Name);
return displayName;
}
return displayName;
}

// Finally falling back to name
return claimsPrincipal.FindFirstValue(ClaimConstants.Name);
}
}
}
7 changes: 3 additions & 4 deletions Microsoft.Identity.Web/Extensions.cs
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
using System;
using System.Collections.Generic;
using System.Text;
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

namespace Microsoft.Identity.Web
{
/// <summary>
/// Extension methods that don't fit in any other class
/// Extension methods
/// </summary>
public static class Extensions
{
Expand Down
5 changes: 4 additions & 1 deletion Microsoft.Identity.Web/HttpContextExtensions.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
using Microsoft.AspNetCore.Http;
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using Microsoft.AspNetCore.Http;
using System.IdentityModel.Tokens.Jwt;

namespace Microsoft.Identity.Web
Expand Down
8 changes: 4 additions & 4 deletions Microsoft.Identity.Web/ITokenAcquisition.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,13 @@ public interface ITokenAcquisition

/// <summary>
/// Typically used from an ASP.NET Core Web App or Web API controller, this method gets an access token
/// for a downstream API on behalf of the user account which claims are provided in the <see cref="HttpContext.User"/>
/// for a downstream API on behalf of the user account in which claims are provided in the <see cref="HttpContext.User"/>
/// member of the <paramref name="context"/> parameter
/// </summary>
/// <param name="context">HttpContext associated with the Controller or auth operation</param>
/// <param name="scopes">Scopes to request for the downstream API to call</param>
/// <param name="tenantId">Enables to override the tenant/account for the same identity. This is useful in the
/// cases where a given account is guest in other tenants, and you want to acquire tokens for a specific tenant</param>
/// cases where a given account is a guest in other tenants, and you want to acquire tokens for a specific tenant</param>
/// <returns>An access token to call on behalf of the user, the downstream API characterized by its scopes</returns>
Task<string> GetAccessTokenOnBehalfOfUserAsync(IEnumerable<string> scopes, string tenantId = null);

Expand All @@ -66,8 +66,8 @@ public interface ITokenAcquisition

/// <summary>
/// Used in Web APIs (which therefore cannot have an interaction with the user).
/// Replies to the client through the HttpReponse by sending a 403 (forbidden) and populating wwwAuthenticateHeaders so that
/// the client can trigger an interaction with the user so that the user consents to more scopes
/// Replies to the client through the HttpResponse by sending a 403 (forbidden) and populating wwwAuthenticateHeaders so that
/// the client can trigger an interaction with the user so the user can consent to more scopes
/// </summary>
/// <param name="scopes">Scopes to consent to</param>
/// <param name="msalSeviceException"><see cref="MsalUiRequiredException"/> triggering the challenge</param>
Expand Down
Loading