Skip to content

Commit

Permalink
Implemented C# 8 Nullable Reference Type standard. Includes additiona…
Browse files Browse the repository at this point in the history
…l null checks for public method parameters. Minor comment fixes.
  • Loading branch information
pmaytak committed Jun 25, 2020
1 parent 35d83f6 commit 9a9fb77
Show file tree
Hide file tree
Showing 51 changed files with 471 additions and 313 deletions.
14 changes: 7 additions & 7 deletions .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -467,9 +467,6 @@ dotnet_diagnostic.SA1309.severity = none
# CA1031: Do not catch general exception types
dotnet_diagnostic.CA1031.severity = none

# CA1062: Validate arguments of public methods
dotnet_diagnostic.CA1062.severity = none

# CA1303: Do not pass literals as localized parameters
dotnet_diagnostic.CA1303.severity = none

Expand Down Expand Up @@ -527,20 +524,23 @@ dotnet_diagnostic.SA1614.severity = none
# SA1615: Element return value should be documented
dotnet_diagnostic.SA1615.severity = none

# SA1616: Element return value documentation should have text
dotnet_diagnostic.SA1616.severity = none

# SA1623: Property summary documentation should match accessors
dotnet_diagnostic.SA1623.severity = none

# SA1642: Constructor summary documentation should begin with standard text
dotnet_diagnostic.SA1642.severity = none

# SA1649: File name should match first type name
## SA1649: File name should match first type name
dotnet_diagnostic.SA1649.severity = none
#########################################################
#########################################################

# SA1011: Closing square brackets should be spaced correctly
dotnet_diagnostic.SA1011.severity = none

# SA1009: Closing parenthesis should be spaced correctly
dotnet_diagnostic.SA1009.severity = none

[*.{cpp,h,in}]
curly_bracket_next_line = true
indent_brace_style = Allman
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ public class ErrorModel : PageModel
{
/// <summary>
/// This API supports infrastructure and is not intended to be used
/// directly from your code.This API may change or be removed in future releases.
/// directly from your code. This API may change or be removed in future releases.
/// </summary>
public string RequestId { get; set; }
public string? RequestId { get; set; }

/// <summary>
/// This API supports infrastructure and is not intended to be used
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@ public class SignedOutModel : PageModel
/// <summary>
/// Method handling the HTTP GET method.
/// </summary>
/// <returns></returns>
/// <returns>A Sign Out page or Home page.</returns>
public IActionResult OnGet()
{
if (User.Identity.IsAuthenticated)
if (User?.Identity?.IsAuthenticated ?? false)
{
return LocalRedirect("~/");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,10 @@
<PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.0.0" PrivateAssets="All" />
</ItemGroup>
<PropertyGroup>
<TargetFrameworks>netcoreapp3.1; net5.0</TargetFrameworks>
<TargetFrameworks>netcoreapp3.1; net5.0</TargetFrameworks>
<AddRazorSupportForMvc>true</AddRazorSupportForMvc>
<GenerateDocumentationFile>true</GenerateDocumentationFile>
<Nullable>enable</Nullable>
</PropertyGroup>
<PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|AnyCPU'">
<DocumentationFile>C:\gh\microsoft-identity-web\src\Microsoft.Identity.Web.UI\Microsoft.Identity.Web.UI.xml</DocumentationFile>
Expand Down
34 changes: 22 additions & 12 deletions src/Microsoft.Identity.Web/AccountExtensions.cs
Original file line number Diff line number Diff line change
@@ -1,36 +1,46 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using System;
using System.Security.Claims;
using Microsoft.Identity.Client;

namespace Microsoft.Identity.Web
{
/// <summary>
/// Extension methods dealing with IAccount instances.
/// Extension methods for <see cref="IAccount"/>.
/// </summary>
public static class AccountExtensions
{
/// <summary>
/// Creates the <see cref="ClaimsPrincipal"/> from the values found
/// in an <see cref="IAccount"/>.
/// </summary>
/// <param name="account">The IAccount instance.</param>
/// <returns>A <see cref="ClaimsPrincipal"/> built from IAccount.</returns>
/// <param name="account">The <see cref="IAccount"/> instance.</param>
/// <returns>A <see cref="ClaimsPrincipal"/> built from <see cref="IAccount"/>.</returns>
public static ClaimsPrincipal ToClaimsPrincipal(this IAccount account)
{
if (account != null)
if (account == null)
{
return new ClaimsPrincipal(
new ClaimsIdentity(new Claim[]
{
new Claim(ClaimConstants.Oid, account.HomeAccountId?.ObjectId),
new Claim(ClaimConstants.Tid, account.HomeAccountId?.TenantId),
new Claim(ClaimTypes.Upn, account.Username),
}));
throw new ArgumentNullException(nameof(account));
}

return null;
ClaimsIdentity identity = new ClaimsIdentity(new Claim[]
{
new Claim(ClaimTypes.Upn, account.Username),
});

if (!string.IsNullOrEmpty(account.HomeAccountId?.ObjectId))
{
identity.AddClaim(new Claim(ClaimConstants.Oid, account.HomeAccountId.ObjectId));
}

if (!string.IsNullOrEmpty(account.HomeAccountId?.TenantId))
{
identity.AddClaim(new Claim(ClaimConstants.Tid, account.HomeAccountId.TenantId));
}

return new ClaimsPrincipal(identity);
}
}
}
4 changes: 2 additions & 2 deletions src/Microsoft.Identity.Web/AuthorityHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ internal static class AuthorityHelpers
{
internal static string BuildAuthority(MicrosoftIdentityOptions options)
{
var baseUri = new Uri(options.Instance);
var pathBase = baseUri.PathAndQuery.TrimEnd('/');
Uri baseUri = new Uri(options.Instance);
string pathBase = baseUri.PathAndQuery.TrimEnd('/');
var domain = options.Domain;
var tenantId = options.TenantId;

Expand Down
38 changes: 19 additions & 19 deletions src/Microsoft.Identity.Web/AuthorizeForScopesAttribute.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,38 +32,37 @@ public class AuthorizeForScopesAttribute : ExceptionFilterAttribute
/// <summary>
/// Scopes to request.
/// </summary>
public string[] Scopes { get; set; }
public string[]? Scopes { get; set; }

/// <summary>
/// Key section on the configuration file that holds the scope value.
/// </summary>
public string ScopeKeySection { get; set; }
public string? ScopeKeySection { get; set; }

/// <summary>
/// Handles the MsalUiRequiredException.
/// Handles the <see cref="MsalUiRequiredException"/>.
/// </summary>
/// <param name="context">Context provided by ASP.NET Core.</param>
public override void OnException(ExceptionContext context)
{
// Do not re-use the attribute param Scopes. For more info: https://github.com/Azure-Samples/active-directory-aspnetcore-webapp-openidconnect-v2/issues/273
string[] incrementalConsentScopes = Array.Empty<string>();
MsalUiRequiredException msalUiRequiredException = context.Exception as MsalUiRequiredException;

if (msalUiRequiredException == null)
if (context != null)
{
msalUiRequiredException = context.Exception?.InnerException as MsalUiRequiredException;
}
MsalUiRequiredException? msalUiRequiredException =
(context.Exception as MsalUiRequiredException)
?? (context.Exception?.InnerException as MsalUiRequiredException);

if (msalUiRequiredException != null)
{
if (CanBeSolvedByReSignInOfUser(msalUiRequiredException))
if (msalUiRequiredException != null &&
CanBeSolvedByReSignInOfUser(msalUiRequiredException))
{
// the users cannot provide both scopes and ScopeKeySection at the same time
if (!string.IsNullOrWhiteSpace(ScopeKeySection) && Scopes != null && Scopes.Length > 0)
{
throw new InvalidOperationException($"Either provide the '{nameof(ScopeKeySection)}' or the '{nameof(Scopes)}' to the 'AuthorizeForScopes'.");
}

// Do not re-use the attribute param Scopes. For more info: https://github.com/Azure-Samples/active-directory-aspnetcore-webapp-openidconnect-v2/issues/273
string[]? incrementalConsentScopes;

// If the user wishes us to pick the Scopes from a particular config setting.
if (!string.IsNullOrWhiteSpace(ScopeKeySection))
{
Expand All @@ -77,7 +76,7 @@ public override void OnException(ExceptionContext context)

incrementalConsentScopes = new string[] { configuration.GetValue<string>(ScopeKeySection) };

if (Scopes != null && Scopes.Length > 0 && incrementalConsentScopes != null && incrementalConsentScopes.Length > 0)
if (Scopes != null && Scopes.Length > 0 && incrementalConsentScopes.Length > 0)
{
throw new InvalidOperationException("no scopes provided in scopes...");
}
Expand All @@ -87,7 +86,7 @@ public override void OnException(ExceptionContext context)
incrementalConsentScopes = Scopes;
}

var properties = BuildAuthenticationPropertiesForIncrementalConsent(incrementalConsentScopes, msalUiRequiredException, context.HttpContext);
AuthenticationProperties properties = BuildAuthenticationPropertiesForIncrementalConsent(incrementalConsentScopes, msalUiRequiredException, context.HttpContext);
context.Result = new ChallengeResult(properties);
}
}
Expand All @@ -107,17 +106,18 @@ private bool CanBeSolvedByReSignInOfUser(MsalUiRequiredException ex)
}

/// <summary>
/// Build Authentication properties needed for 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>
/// <param name="ex"><see cref="MsalUiRequiredException"/> instance.</param>
/// <param name="context">Current HTTP context in the pipeline.</param>
/// <returns>AuthenticationProperties.</returns>
private AuthenticationProperties BuildAuthenticationPropertiesForIncrementalConsent(
string[] scopes,
string[]? scopes,
MsalUiRequiredException ex,
HttpContext context)
{
scopes ??= new string[0];
var properties = new AuthenticationProperties();

// Set the scopes, including the scopes that ADAL.NET / MSAL.NET need for the token cache
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,12 @@ public Task OnRemoteFailure(RemoteFailureContext context)
return Task.CompletedTask;
}

private string BuildIssuerAddress(RedirectContext context, string defaultUserFlow, string userFlow)
private string BuildIssuerAddress(RedirectContext context, string? defaultUserFlow, string userFlow)
{
if (!_userFlowToIssuerAddress.TryGetValue(userFlow, out var issuerAddress))
{
_userFlowToIssuerAddress[userFlow] = context.ProtocolMessage.IssuerAddress.ToLowerInvariant()
.Replace($"/{defaultUserFlow.ToLowerInvariant()}/", $"/{userFlow.ToLowerInvariant()}/");
.Replace($"/{defaultUserFlow?.ToLowerInvariant()}/", $"/{userFlow.ToLowerInvariant()}/");
}

return _userFlowToIssuerAddress[userFlow];
Expand Down
5 changes: 3 additions & 2 deletions src/Microsoft.Identity.Web/Base64UrlHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT License.

using System;
using System.Diagnostics.CodeAnalysis;
using System.Globalization;
using System.Text;

Expand All @@ -23,7 +24,7 @@ internal static class Base64UrlHelpers
// * the 62nd and 63rd regular base64 encoding characters ('+' and '/') are replace with ('-' and '_')
// The changes make the encoding alphabet file and URL safe
// See RFC4648, section 5 for more info
public static string Encode(string arg)
public static string? Encode(string arg)
{
if (arg == null)
{
Expand Down Expand Up @@ -68,7 +69,7 @@ public static byte[] DecodeToBytes(string arg)
return Convert.FromBase64String(s); // Standard base64 decoder
}

internal static string Encode(byte[] arg)
internal static string? Encode(byte[] arg)
{
if (arg == null)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public static CertificateDescription FromBase64Encoded(string base64EncodedValue
/// <param name="path">Path were to find the certificate file.</param>
/// <param name="password">Certificate password.</param>
/// <returns>A certificate description.</returns>
public static CertificateDescription FromPath(string path, string password = null)
public static CertificateDescription FromPath(string path, string? password = null)
{
return new CertificateDescription
{
Expand Down Expand Up @@ -131,7 +131,7 @@ public static CertificateDescription FromStoreWithDistinguishedName(
/// this value is the path to the certificate in the cert store, for instance <c>CurrentUser/My</c>.</item>
/// </list>
/// </summary>
internal string Container
internal string? Container
{
get
{
Expand Down Expand Up @@ -179,43 +179,43 @@ internal string Container
/// <summary>
/// URL of the Key Vault for instance https://msidentitywebsamples.vault.azure.net.
/// </summary>
public string KeyVaultUrl { get; set; }
public string? KeyVaultUrl { get; set; }

/// <summary>
/// Certificate store path, for instance "CurrentUser/My".
/// </summary>
/// <remarks>This property should only be used in conjunction with DistinguishName or Thumbprint.</remarks>
public string CertificateStorePath { get; set; }
public string? CertificateStorePath { get; set; }

/// <summary>
/// Certificate distinguished name.
/// </summary>
public string CertificateDistinguishedName { get; set; }
public string? CertificateDistinguishedName { get; set; }

/// <summary>
/// Name of the certificate in Key Vault.
/// </summary>
public string KeyVaultCertificateName { get; set; }
public string? KeyVaultCertificateName { get; set; }

/// <summary>
/// Certificate thumbprint.
/// </summary>
public string CertificateThumbprint { get; set; }
public string? CertificateThumbprint { get; set; }

/// <summary>
/// Path on disk to the certificate.
/// </summary>
public string CertificateDiskPath { get; set; }
public string? CertificateDiskPath { get; set; }

/// <summary>
/// Path on disk to the certificate password.
/// </summary>
public string CertificatePassword { get; set; }
public string? CertificatePassword { get; set; }

/// <summary>
/// Base64 encoded certificate value.
/// </summary>
public string Base64EncodedValue { get; set; }
public string? Base64EncodedValue { get; set; }

/// <summary>
/// Reference to the certificate or value.
Expand All @@ -232,7 +232,7 @@ internal string Container
/// <item>If <see cref="SourceType"/> equals <see cref="CertificateSource.StoreWithThumbprint"/>,
/// this value is the thumbprint.</item>
/// </list>
internal string ReferenceOrValue
internal string? ReferenceOrValue
{
get
{
Expand Down Expand Up @@ -284,6 +284,6 @@ internal string ReferenceOrValue
/// The certificate, either provided directly in code
/// or loaded from the description.
/// </summary>
public X509Certificate2 Certificate { get; internal set; }
public X509Certificate2? Certificate { get; internal set; }
}
}
Loading

0 comments on commit 9a9fb77

Please sign in to comment.