Skip to content

Commit 794daae

Browse files
jennyf19jmprieur
authored andcommitted
fix some comments and namings (#244)
1 parent 81126a6 commit 794daae

15 files changed

+111
-104
lines changed

Microsoft.Identity.Web/AccountExtensions.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@ public static ClaimsPrincipal ToClaimsPrincipal(this IAccount account)
2424
return new ClaimsPrincipal(
2525
new ClaimsIdentity(new Claim[]
2626
{
27-
new Claim(ClaimConstants.Oid, account.HomeAccountId.ObjectId),
28-
new Claim(ClaimConstants.Tid, account.HomeAccountId.TenantId),
27+
new Claim(ClaimConstants.Oid, account.HomeAccountId?.ObjectId),
28+
new Claim(ClaimConstants.Tid, account.HomeAccountId?.TenantId),
2929
new Claim(ClaimTypes.Upn, account.Username)
3030
})
3131
);

Microsoft.Identity.Web/AuthorizeForScopesAttribute.cs

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,8 @@
1515

1616
namespace Microsoft.Identity.Web
1717
{
18-
// TODO: rename to EnsureScopesAttribute ? or MsalAuthorizeForScopesAttribute or AuthorizeForScopesAttribute
19-
2018
/// <summary>
21-
/// Filter used on a controller action to trigger an incremental consent.
19+
/// Filter used on a controller action to trigger incremental consent.
2220
/// </summary>
2321
/// <example>
2422
/// The following controller action will trigger
@@ -42,7 +40,7 @@ public class AuthorizeForScopesAttribute : ExceptionFilterAttribute
4240
public string ScopeKeySection { get; set; }
4341

4442
/// <summary>
45-
/// Handles the MsaUiRequiredExeception
43+
/// Handles the MsalUiRequiredException
4644
/// </summary>
4745
/// <param name="context">Context provided by ASP.NET Core</param>
4846
public override void OnException(ExceptionContext context)
@@ -55,7 +53,7 @@ public override void OnException(ExceptionContext context)
5553

5654
if (msalUiRequiredException != null)
5755
{
58-
if (CanBeSolvedByReSignInUser(msalUiRequiredException))
56+
if (CanBeSolvedByReSignInOfUser(msalUiRequiredException))
5957
{
6058
// the users cannot provide both scopes and ScopeKeySection at the same time
6159
if (!string.IsNullOrWhiteSpace(ScopeKeySection) && Scopes != null && Scopes.Length > 0)
@@ -85,7 +83,7 @@ public override void OnException(ExceptionContext context)
8583
base.OnException(context);
8684
}
8785

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

9997
/// <summary>
100-
/// Build Authentication properties needed for an incremental consent.
98+
/// Build Authentication properties needed for incremental consent.
10199
/// </summary>
102100
/// <param name="scopes">Scopes to request</param>
103101
/// <param name="ex">MsalUiRequiredException instance</param>
104102
/// <param name="context">current http context in the pipeline</param>
105103
/// <returns>AuthenticationProperties</returns>
106104
private AuthenticationProperties BuildAuthenticationPropertiesForIncrementalConsent(
107-
string[] scopes, MsalUiRequiredException ex, HttpContext context)
105+
string[] scopes,
106+
MsalUiRequiredException ex,
107+
HttpContext context)
108108
{
109109
var properties = new AuthenticationProperties();
110110

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

117119
// Attempts to set the login_hint to avoid the logged-in user to be presented with an account selection dialog
118120
var loginHint = context.User.GetLoginHint();

Microsoft.Identity.Web/ClaimsPrincipalExtensions.cs

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ public static string GetMsalAccountId(this ClaimsPrincipal claimsPrincipal)
3434
/// <summary>
3535
/// Gets the unique object ID associated with the <see cref="ClaimsPrincipal"/>
3636
/// </summary>
37-
/// <param name="claimsPrincipal">the <see cref="ClaimsPrincipal"/> from which to retrieve the unique object id</param>
37+
/// <param name="claimsPrincipal">the <see cref="ClaimsPrincipal"/> from which to retrieve the unique object ID</param>
3838
/// <remarks>This method returns the object ID both in case the developer has enabled or not claims mapping</remarks>
3939
/// <returns>Unique object ID of the identity, or <c>null</c> if it cannot be found</returns>
4040
public static string GetObjectId(this ClaimsPrincipal claimsPrincipal)
@@ -50,9 +50,9 @@ public static string GetObjectId(this ClaimsPrincipal claimsPrincipal)
5050
/// <summary>
5151
/// Gets the Tenant ID associated with the <see cref="ClaimsPrincipal"/>
5252
/// </summary>
53-
/// <param name="claimsPrincipal">the <see cref="ClaimsPrincipal"/> from which to retrieve the tenant id</param>
53+
/// <param name="claimsPrincipal">the <see cref="ClaimsPrincipal"/> from which to retrieve the tenant ID</param>
5454
/// <returns>Tenant ID of the identity, or <c>null</c> if it cannot be found</returns>
55-
/// <remarks>This method returns the object ID both in case the developer has enabled or not claims mapping</remarks>
55+
/// <remarks>This method returns the tenant ID both in case the developer has enabled or not claims mapping</remarks>
5656
public static string GetTenantId(this ClaimsPrincipal claimsPrincipal)
5757
{
5858
string tenantId = claimsPrincipal.FindFirstValue(ClaimConstants.Tid);
@@ -95,27 +95,29 @@ public static string GetDomainHint(this ClaimsPrincipal claimsPrincipal)
9595
/// Get the display name for the signed-in user, from the <see cref="ClaimsPrincipal"/>
9696
/// </summary>
9797
/// <param name="claimsPrincipal">Claims about the user/account</param>
98-
/// <returns>A string containing the display name for the user, as brought by Azure AD (v1.0) and Microsoft identity platform (v2.0) tokens,
98+
/// <returns>A string containing the display name for the user, as determined by Azure AD (v1.0) and Microsoft identity platform (v2.0) tokens,
9999
/// or <c>null</c> if the claims cannot be found</returns>
100100
/// <remarks>See https://docs.microsoft.com/azure/active-directory/develop/id-tokens#payload-claims </remarks>
101101
public static string GetDisplayName(this ClaimsPrincipal claimsPrincipal)
102102
{
103-
// Use the claims in an Microsoft identity platform token first
103+
// Use the claims in a Microsoft identity platform token first
104104
string displayName = claimsPrincipal.FindFirstValue(ClaimConstants.PreferredUserName);
105105

106-
// Otherwise fall back to the claims in an Azure AD v1.0 token
107-
if (string.IsNullOrWhiteSpace(displayName))
106+
if (!string.IsNullOrWhiteSpace(displayName))
108107
{
109-
displayName = claimsPrincipal.FindFirstValue(ClaimsIdentity.DefaultNameClaimType);
108+
return displayName;
110109
}
111110

112-
// Finally falling back to name
113-
if (string.IsNullOrWhiteSpace(displayName))
111+
// Otherwise fall back to the claims in an Azure AD v1.0 token
112+
displayName = claimsPrincipal.FindFirstValue(ClaimsIdentity.DefaultNameClaimType);
113+
114+
if (!string.IsNullOrWhiteSpace(displayName))
114115
{
115-
displayName = claimsPrincipal.FindFirstValue(ClaimConstants.Name);
116+
return displayName;
116117
}
117-
return displayName;
118-
}
119118

119+
// Finally falling back to name
120+
return claimsPrincipal.FindFirstValue(ClaimConstants.Name);
121+
}
120122
}
121123
}

Microsoft.Identity.Web/Extensions.cs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
1-
using System;
2-
using System.Collections.Generic;
3-
using System.Text;
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
43

54
namespace Microsoft.Identity.Web
65
{
76
/// <summary>
8-
/// Extension methods that don't fit in any other class
7+
/// Extension methods
98
/// </summary>
109
public static class Extensions
1110
{

Microsoft.Identity.Web/HttpContextExtensions.cs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
1-
using Microsoft.AspNetCore.Http;
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
using Microsoft.AspNetCore.Http;
25
using System.IdentityModel.Tokens.Jwt;
36

47
namespace Microsoft.Identity.Web

Microsoft.Identity.Web/ITokenAcquisition.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,13 +46,13 @@ public interface ITokenAcquisition
4646

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

@@ -66,8 +66,8 @@ public interface ITokenAcquisition
6666

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

0 commit comments

Comments
 (0)