From 9a9fb7716329b644ef9e3e731b4c02c51664d46e Mon Sep 17 00:00:00 2001 From: pmaytak <34331512+pmaytak@users.noreply.github.com> Date: Tue, 23 Jun 2020 00:15:04 -0700 Subject: [PATCH] Implemented C# 8 Nullable Reference Type standard. Includes additional null checks for public method parameters. Minor comment fixes. --- .editorconfig | 14 ++-- .../Pages/Account/Error.cshtml.cs | 4 +- .../Pages/Account/SignedOut.cshtml.cs | 4 +- .../Microsoft.Identity.Web.UI.csproj | 3 +- .../AccountExtensions.cs | 34 +++++--- .../AuthorityHelpers.cs | 4 +- .../AuthorizeForScopesAttribute.cs | 38 ++++----- .../AzureADB2COpenIDConnectEventHandlers.cs | 4 +- .../Base64UrlHelpers.cs | 5 +- .../CertificateDescription.cs | 24 +++--- .../DefaultCertificateLoader.cs | 34 ++++---- .../ClaimsPrincipalExtensions.cs | 82 +++++++++++++------ .../ClaimsPrincipalFactory.cs | 18 ++-- src/Microsoft.Identity.Web/ClientInfo.cs | 9 +- .../CookiePolicyOptionsExtensions.cs | 81 ++++++++++-------- .../HttpContextExtensions.cs | 12 +-- .../ITokenAcquisition.cs | 4 +- .../ITokenAcquisitionInternal.cs | 3 +- .../IssuerConfigurationRetriever.cs | 12 +-- .../InstanceDiscovery/IssuerMetadata.cs | 6 +- .../InstanceDiscovery/Metadata.cs | 6 +- .../Microsoft.Identity.Web.csproj | 9 +- .../MicrosoftIdentityOptions.cs | 20 ++--- .../MicrosoftIdentityOptionsValidation.cs | 5 +- ...-LegacyTokenDecryptCertificateParameter.cs | 2 +- ...e-WebApiAuthenticationBuilderExtensions.cs | 6 +- ...olete-WebApiServiceCollectionExtensions.cs | 4 +- ...olete-WebAppServiceCollectionExtensions.cs | 2 +- .../Resource/AadIssuerValidator.cs | 18 ++-- .../IJwtBearerMiddlewareDiagnostics.cs | 8 +- .../IOpenIdConnectMiddlewareDiagnostics.cs | 6 +- .../JwtBearerMiddlewareDiagnostics.cs | 13 ++- .../OpenIdConnectMiddlewareDiagnostics.cs | 26 +++--- .../Resource/RegisterValidAudience.cs | 9 +- .../ScopesRequiredHttpContextExtensions.cs | 9 +- .../ServiceCollectionExtensions.cs | 6 ++ .../TokenAcquisition.cs | 70 ++++++++-------- .../DistributedTokenCacheAdapterExtension.cs | 17 +++- .../MsalDistributedTokenCacheAdapter.cs | 6 ++ .../IMsalTokenCacheProvider.cs | 4 +- .../InMemoryTokenCacheProviderExtension.cs | 8 +- .../MsalAbstractTokenCacheProvider.cs | 30 ++++--- .../SessionTokenCacheProviderExtension.cs | 16 ++++ .../WebApiAuthenticationBuilderExtensions.cs | 4 +- ...lsWebApiAuthenticationBuilderExtensions.cs | 12 ++- .../WebAppAuthenticationBuilderExtensions.cs | 17 +++- ...lsWebApiAuthenticationBuilderExtensions.cs | 34 +++++--- tests/.editorconfig | 6 ++ .../AccountExtensionsTests.cs | 12 +-- .../IssuerConfigurationRetrieverTests.cs | 2 +- ...copesRequiredHttpContextExtensionsTests.cs | 2 +- 51 files changed, 471 insertions(+), 313 deletions(-) diff --git a/.editorconfig b/.editorconfig index e1e371638..165429393 100644 --- a/.editorconfig +++ b/.editorconfig @@ -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 @@ -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 diff --git a/src/Microsoft.Identity.Web.UI/Areas/MicrosoftIdentity/Pages/Account/Error.cshtml.cs b/src/Microsoft.Identity.Web.UI/Areas/MicrosoftIdentity/Pages/Account/Error.cshtml.cs index ebf27f8f6..91ace06f4 100644 --- a/src/Microsoft.Identity.Web.UI/Areas/MicrosoftIdentity/Pages/Account/Error.cshtml.cs +++ b/src/Microsoft.Identity.Web.UI/Areas/MicrosoftIdentity/Pages/Account/Error.cshtml.cs @@ -17,9 +17,9 @@ public class ErrorModel : PageModel { /// /// 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. /// - public string RequestId { get; set; } + public string? RequestId { get; set; } /// /// This API supports infrastructure and is not intended to be used diff --git a/src/Microsoft.Identity.Web.UI/Areas/MicrosoftIdentity/Pages/Account/SignedOut.cshtml.cs b/src/Microsoft.Identity.Web.UI/Areas/MicrosoftIdentity/Pages/Account/SignedOut.cshtml.cs index 0168aa7c1..df60187f0 100644 --- a/src/Microsoft.Identity.Web.UI/Areas/MicrosoftIdentity/Pages/Account/SignedOut.cshtml.cs +++ b/src/Microsoft.Identity.Web.UI/Areas/MicrosoftIdentity/Pages/Account/SignedOut.cshtml.cs @@ -16,10 +16,10 @@ public class SignedOutModel : PageModel /// /// Method handling the HTTP GET method. /// - /// + /// A Sign Out page or Home page. public IActionResult OnGet() { - if (User.Identity.IsAuthenticated) + if (User?.Identity?.IsAuthenticated ?? false) { return LocalRedirect("~/"); } diff --git a/src/Microsoft.Identity.Web.UI/Microsoft.Identity.Web.UI.csproj b/src/Microsoft.Identity.Web.UI/Microsoft.Identity.Web.UI.csproj index 613fe2f23..bcf0a31ed 100644 --- a/src/Microsoft.Identity.Web.UI/Microsoft.Identity.Web.UI.csproj +++ b/src/Microsoft.Identity.Web.UI/Microsoft.Identity.Web.UI.csproj @@ -42,9 +42,10 @@ - netcoreapp3.1; net5.0 + netcoreapp3.1; net5.0 true true + enable C:\gh\microsoft-identity-web\src\Microsoft.Identity.Web.UI\Microsoft.Identity.Web.UI.xml diff --git a/src/Microsoft.Identity.Web/AccountExtensions.cs b/src/Microsoft.Identity.Web/AccountExtensions.cs index fb5f8d93e..605e77913 100644 --- a/src/Microsoft.Identity.Web/AccountExtensions.cs +++ b/src/Microsoft.Identity.Web/AccountExtensions.cs @@ -1,13 +1,14 @@ // 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 { /// - /// Extension methods dealing with IAccount instances. + /// Extension methods for . /// public static class AccountExtensions { @@ -15,22 +16,31 @@ public static class AccountExtensions /// Creates the from the values found /// in an . /// - /// The IAccount instance. - /// A built from IAccount. + /// The instance. + /// A built from . 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); } } } diff --git a/src/Microsoft.Identity.Web/AuthorityHelpers.cs b/src/Microsoft.Identity.Web/AuthorityHelpers.cs index a39a32bc7..745bd275c 100644 --- a/src/Microsoft.Identity.Web/AuthorityHelpers.cs +++ b/src/Microsoft.Identity.Web/AuthorityHelpers.cs @@ -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; diff --git a/src/Microsoft.Identity.Web/AuthorizeForScopesAttribute.cs b/src/Microsoft.Identity.Web/AuthorizeForScopesAttribute.cs index a128d33ab..79ceb4ea0 100644 --- a/src/Microsoft.Identity.Web/AuthorizeForScopesAttribute.cs +++ b/src/Microsoft.Identity.Web/AuthorizeForScopesAttribute.cs @@ -32,31 +32,27 @@ public class AuthorizeForScopesAttribute : ExceptionFilterAttribute /// /// Scopes to request. /// - public string[] Scopes { get; set; } + public string[]? Scopes { get; set; } /// /// Key section on the configuration file that holds the scope value. /// - public string ScopeKeySection { get; set; } + public string? ScopeKeySection { get; set; } /// - /// Handles the MsalUiRequiredException. + /// Handles the . /// /// Context provided by ASP.NET Core. 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(); - 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) @@ -64,6 +60,9 @@ public override void OnException(ExceptionContext context) 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)) { @@ -77,7 +76,7 @@ public override void OnException(ExceptionContext context) incrementalConsentScopes = new string[] { configuration.GetValue(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..."); } @@ -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); } } @@ -107,17 +106,18 @@ private bool CanBeSolvedByReSignInOfUser(MsalUiRequiredException ex) } /// - /// Build Authentication properties needed for incremental consent. + /// Build authentication properties needed for incremental consent. /// /// Scopes to request. - /// MsalUiRequiredException instance. - /// current HTTP context in the pipeline. + /// instance. + /// Current HTTP context in the pipeline. /// AuthenticationProperties. 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 diff --git a/src/Microsoft.Identity.Web/AzureADB2COpenIDConnectEventHandlers.cs b/src/Microsoft.Identity.Web/AzureADB2COpenIDConnectEventHandlers.cs index c5a704d02..dba3b0c3d 100644 --- a/src/Microsoft.Identity.Web/AzureADB2COpenIDConnectEventHandlers.cs +++ b/src/Microsoft.Identity.Web/AzureADB2COpenIDConnectEventHandlers.cs @@ -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]; diff --git a/src/Microsoft.Identity.Web/Base64UrlHelpers.cs b/src/Microsoft.Identity.Web/Base64UrlHelpers.cs index cd7068860..76f3a75ce 100644 --- a/src/Microsoft.Identity.Web/Base64UrlHelpers.cs +++ b/src/Microsoft.Identity.Web/Base64UrlHelpers.cs @@ -2,6 +2,7 @@ // Licensed under the MIT License. using System; +using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.Text; @@ -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) { @@ -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) { diff --git a/src/Microsoft.Identity.Web/CertificateManagement/CertificateDescription.cs b/src/Microsoft.Identity.Web/CertificateManagement/CertificateDescription.cs index dcf9d2005..7b8f21bdd 100644 --- a/src/Microsoft.Identity.Web/CertificateManagement/CertificateDescription.cs +++ b/src/Microsoft.Identity.Web/CertificateManagement/CertificateDescription.cs @@ -61,7 +61,7 @@ public static CertificateDescription FromBase64Encoded(string base64EncodedValue /// Path were to find the certificate file. /// Certificate password. /// A certificate description. - public static CertificateDescription FromPath(string path, string password = null) + public static CertificateDescription FromPath(string path, string? password = null) { return new CertificateDescription { @@ -131,7 +131,7 @@ public static CertificateDescription FromStoreWithDistinguishedName( /// this value is the path to the certificate in the cert store, for instance CurrentUser/My. /// /// - internal string Container + internal string? Container { get { @@ -179,43 +179,43 @@ internal string Container /// /// URL of the Key Vault for instance https://msidentitywebsamples.vault.azure.net. /// - public string KeyVaultUrl { get; set; } + public string? KeyVaultUrl { get; set; } /// /// Certificate store path, for instance "CurrentUser/My". /// /// This property should only be used in conjunction with DistinguishName or Thumbprint. - public string CertificateStorePath { get; set; } + public string? CertificateStorePath { get; set; } /// /// Certificate distinguished name. /// - public string CertificateDistinguishedName { get; set; } + public string? CertificateDistinguishedName { get; set; } /// /// Name of the certificate in Key Vault. /// - public string KeyVaultCertificateName { get; set; } + public string? KeyVaultCertificateName { get; set; } /// /// Certificate thumbprint. /// - public string CertificateThumbprint { get; set; } + public string? CertificateThumbprint { get; set; } /// /// Path on disk to the certificate. /// - public string CertificateDiskPath { get; set; } + public string? CertificateDiskPath { get; set; } /// /// Path on disk to the certificate password. /// - public string CertificatePassword { get; set; } + public string? CertificatePassword { get; set; } /// /// Base64 encoded certificate value. /// - public string Base64EncodedValue { get; set; } + public string? Base64EncodedValue { get; set; } /// /// Reference to the certificate or value. @@ -232,7 +232,7 @@ internal string Container /// If equals , /// this value is the thumbprint. /// - internal string ReferenceOrValue + internal string? ReferenceOrValue { get { @@ -284,6 +284,6 @@ internal string ReferenceOrValue /// The certificate, either provided directly in code /// or loaded from the description. /// - public X509Certificate2 Certificate { get; internal set; } + public X509Certificate2? Certificate { get; internal set; } } } diff --git a/src/Microsoft.Identity.Web/CertificateManagement/DefaultCertificateLoader.cs b/src/Microsoft.Identity.Web/CertificateManagement/DefaultCertificateLoader.cs index 80772bc0b..d4a24804d 100644 --- a/src/Microsoft.Identity.Web/CertificateManagement/DefaultCertificateLoader.cs +++ b/src/Microsoft.Identity.Web/CertificateManagement/DefaultCertificateLoader.cs @@ -27,19 +27,19 @@ public void LoadIfNeeded(CertificateDescription certificateDescription) switch (certificateDescription.SourceType) { case CertificateSource.KeyVault: - certificateDescription.Certificate = LoadFromKeyVault(certificateDescription.Container, certificateDescription.ReferenceOrValue); + certificateDescription.Certificate = LoadFromKeyVault(certificateDescription.Container!, certificateDescription.ReferenceOrValue!); break; case CertificateSource.Base64Encoded: - certificateDescription.Certificate = LoadFromBase64Encoded(certificateDescription.ReferenceOrValue); + certificateDescription.Certificate = LoadFromBase64Encoded(certificateDescription.ReferenceOrValue!); break; case CertificateSource.Path: - certificateDescription.Certificate = LoadFromPath(certificateDescription.Container, certificateDescription.ReferenceOrValue); + certificateDescription.Certificate = LoadFromPath(certificateDescription.Container!, certificateDescription.ReferenceOrValue!); break; case CertificateSource.StoreWithThumbprint: - certificateDescription.Certificate = LoadFromStoreWithThumbprint(certificateDescription.ReferenceOrValue, certificateDescription.Container); + certificateDescription.Certificate = LoadFromStoreWithThumbprint(certificateDescription.ReferenceOrValue!, certificateDescription.Container!); break; case CertificateSource.StoreWithDistinguishedName: - certificateDescription.Certificate = LoadFromStoreWithDistinguishedName(certificateDescription.ReferenceOrValue, certificateDescription.Container); + certificateDescription.Certificate = LoadFromStoreWithDistinguishedName(certificateDescription.ReferenceOrValue!, certificateDescription.Container!); break; default: break; @@ -52,7 +52,7 @@ private static X509Certificate2 LoadFromBase64Encoded(string certificateBase64) byte[] decoded = Convert.FromBase64String(certificateBase64); return new X509Certificate2( decoded, - (string)null, + (string?)null, X509KeyStorageFlags.MachineKeySet | X509KeyStorageFlags.EphemeralKeySet); } @@ -79,7 +79,7 @@ private static X509Certificate2 LoadFromKeyVault(string keyVaultUrl, string cert { return new X509Certificate2( certificate.Cer, - (string)null, + (string?)null, X509KeyStorageFlags.MachineKeySet | X509KeyStorageFlags.EphemeralKeySet); } @@ -106,7 +106,7 @@ private static X509Certificate2 LoadFromKeyVault(string keyVaultUrl, string cert throw new NotSupportedException($"Only PKCS#12 is supported. Found Content-Type: {secret.Properties.ContentType}"); } - private static X509Certificate2 LoadFromStoreWithThumbprint( + private static X509Certificate2? LoadFromStoreWithThumbprint( string certificateThumbprint, string storeDescription = "CurrentUser/My") { @@ -114,7 +114,7 @@ private static X509Certificate2 LoadFromStoreWithThumbprint( StoreName certificateStoreName = StoreName.My; ParseStoreLocationAndName(storeDescription, ref certificateStoreLocation, ref certificateStoreName); - X509Certificate2 cert; + X509Certificate2? cert; using (X509Store x509Store = new X509Store( certificateStoreName, certificateStoreLocation)) @@ -128,13 +128,13 @@ private static X509Certificate2 LoadFromStoreWithThumbprint( return cert; } - private static X509Certificate2 LoadFromStoreWithDistinguishedName(string certificateSubjectDistinguishedName, string storeDescription = "CurrentUser/My") + private static X509Certificate2? LoadFromStoreWithDistinguishedName(string certificateSubjectDistinguishedName, string storeDescription = "CurrentUser/My") { StoreLocation certificateStoreLocation = StoreLocation.CurrentUser; StoreName certificateStoreName = StoreName.My; ParseStoreLocationAndName(storeDescription, ref certificateStoreLocation, ref certificateStoreName); - X509Certificate2 cert; + X509Certificate2? cert; using (X509Store x509Store = new X509Store( certificateStoreName, certificateStoreLocation)) @@ -150,7 +150,7 @@ private static X509Certificate2 LoadFromStoreWithDistinguishedName(string certif private static X509Certificate2 LoadFromPath( string certificateFileName, - string password = null) + string? password = null) { return new X509Certificate2( certificateFileName, @@ -174,11 +174,7 @@ private static void ParseStoreLocationAndName(string storeDescription, ref Store /// /// Find a certificate by criteria. /// - /// - /// - /// - /// - private static X509Certificate2 FindCertificateByCriterium( + private static X509Certificate2? FindCertificateByCriterium( X509Store x509Store, X509FindType identifierCriterium, string certificateIdentifier) @@ -198,12 +194,12 @@ private static X509Certificate2 FindCertificateByCriterium( return cert; } - internal /*for test only*/ static X509Certificate2 LoadFirstCertificate(IEnumerable certificateDescription) + internal /*for test only*/ static X509Certificate2? LoadFirstCertificate(IEnumerable certificateDescription) { DefaultCertificateLoader defaultCertificateLoader = new DefaultCertificateLoader(); CertificateDescription certDescription = certificateDescription.First(); defaultCertificateLoader.LoadIfNeeded(certDescription); - return certDescription?.Certificate; + return certDescription.Certificate; } } } diff --git a/src/Microsoft.Identity.Web/ClaimsPrincipalExtensions.cs b/src/Microsoft.Identity.Web/ClaimsPrincipalExtensions.cs index 4c8a20cce..1689aef3f 100644 --- a/src/Microsoft.Identity.Web/ClaimsPrincipalExtensions.cs +++ b/src/Microsoft.Identity.Web/ClaimsPrincipalExtensions.cs @@ -7,24 +7,24 @@ namespace Microsoft.Identity.Web { /// - /// Extensions around ClaimsPrincipal. + /// Extensions for . /// public static class ClaimsPrincipalExtensions { /// - /// Gets the Account identifier for an MSAL.NET account from a . + /// Gets the account identifier for an MSAL.NET account from a . /// /// Claims principal. /// A string corresponding to an account identifier as defined in . - public static string GetMsalAccountId(this ClaimsPrincipal claimsPrincipal) + public static string? GetMsalAccountId(this ClaimsPrincipal claimsPrincipal) { if (claimsPrincipal == null) { throw new ArgumentNullException(nameof(claimsPrincipal)); } - string uniqueObjectIdentifier = claimsPrincipal.GetHomeObjectId(); - string uniqueTenantIdentifier = claimsPrincipal.GetHomeTenantId(); + string? uniqueObjectIdentifier = claimsPrincipal.GetHomeObjectId(); + string? uniqueTenantIdentifier = claimsPrincipal.GetHomeTenantId(); if (!string.IsNullOrWhiteSpace(uniqueObjectIdentifier) && !string.IsNullOrWhiteSpace(uniqueTenantIdentifier)) { @@ -39,12 +39,17 @@ public static string GetMsalAccountId(this ClaimsPrincipal claimsPrincipal) /// /// Gets the unique object ID associated with the . /// - /// the from which to retrieve the unique object ID. + /// The from which to retrieve the unique object ID. /// This method returns the object ID both in case the developer has enabled or not claims mapping. /// Unique object ID of the identity, or null if it cannot be found. - public static string GetObjectId(this ClaimsPrincipal claimsPrincipal) + public static string? GetObjectId(this ClaimsPrincipal claimsPrincipal) { - string userObjectId = claimsPrincipal.FindFirstValue(ClaimConstants.Oid); + if (claimsPrincipal == null) + { + throw new ArgumentNullException(nameof(claimsPrincipal)); + } + + string? userObjectId = claimsPrincipal.FindFirstValue(ClaimConstants.Oid); if (string.IsNullOrEmpty(userObjectId)) { userObjectId = claimsPrincipal.FindFirstValue(ClaimConstants.ObjectId); @@ -56,12 +61,17 @@ public static string GetObjectId(this ClaimsPrincipal claimsPrincipal) /// /// Gets the Tenant ID associated with the . /// - /// the from which to retrieve the tenant ID. + /// The from which to retrieve the tenant ID. /// Tenant ID of the identity, or null if it cannot be found. /// This method returns the tenant ID both in case the developer has enabled or not claims mapping. - public static string GetTenantId(this ClaimsPrincipal claimsPrincipal) + public static string? GetTenantId(this ClaimsPrincipal claimsPrincipal) { - string tenantId = claimsPrincipal.FindFirstValue(ClaimConstants.Tid); + if (claimsPrincipal == null) + { + throw new ArgumentNullException(nameof(claimsPrincipal)); + } + + string? tenantId = claimsPrincipal.FindFirstValue(ClaimConstants.Tid); if (string.IsNullOrEmpty(tenantId)) { return claimsPrincipal.FindFirstValue(ClaimConstants.TenantId); @@ -74,8 +84,8 @@ public static string GetTenantId(this ClaimsPrincipal claimsPrincipal) /// Gets the login-hint associated with a . /// /// Identity for which to complete the login-hint. - /// login-hint for the identity, or null if it cannot be found. - public static string GetLoginHint(this ClaimsPrincipal claimsPrincipal) + /// The login hint for the identity, or null if it cannot be found. + public static string? GetLoginHint(this ClaimsPrincipal claimsPrincipal) { return GetDisplayName(claimsPrincipal); } @@ -84,14 +94,19 @@ public static string GetLoginHint(this ClaimsPrincipal claimsPrincipal) /// Gets the domain-hint associated with an identity. /// /// Identity for which to compute the domain-hint. - /// domain-hint for the identity, or null if it cannot be found. - public static string GetDomainHint(this ClaimsPrincipal claimsPrincipal) + /// The domain hint for the identity, or null if it cannot be found. + public static string? GetDomainHint(this ClaimsPrincipal claimsPrincipal) { + if (claimsPrincipal == null) + { + throw new ArgumentNullException(nameof(claimsPrincipal)); + } + // Tenant for MSA accounts const string msaTenantId = "9188040d-6c67-4c5b-b112-36a304b66dad"; var tenantId = GetTenantId(claimsPrincipal); - string domainHint = string.IsNullOrWhiteSpace(tenantId) + string? domainHint = string.IsNullOrWhiteSpace(tenantId) ? null : tenantId.Equals(msaTenantId, StringComparison.OrdinalIgnoreCase) ? "consumers" : "organizations"; @@ -105,10 +120,15 @@ public static string GetDomainHint(this ClaimsPrincipal claimsPrincipal) /// A string containing the display name for the user, as determined by Azure AD (v1.0) and Microsoft identity platform (v2.0) tokens, /// or null if the claims cannot be found. /// See https://docs.microsoft.com/azure/active-directory/develop/id-tokens#payload-claims. - public static string GetDisplayName(this ClaimsPrincipal claimsPrincipal) + public static string? GetDisplayName(this ClaimsPrincipal claimsPrincipal) { + if (claimsPrincipal == null) + { + throw new ArgumentNullException(nameof(claimsPrincipal)); + } + // Use the claims in a Microsoft identity platform token first - string displayName = claimsPrincipal.FindFirstValue(ClaimConstants.PreferredUserName); + string? displayName = claimsPrincipal.FindFirstValue(ClaimConstants.PreferredUserName); if (!string.IsNullOrWhiteSpace(displayName)) { @@ -128,13 +148,18 @@ public static string GetDisplayName(this ClaimsPrincipal claimsPrincipal) } /// - /// Gets the user flow id associated with the . + /// Gets the user flow ID associated with the . /// - /// the from which to retrieve the user flow id. - /// User Flow Id of the identity, or null if it cannot be found. - public static string GetUserFlowId(this ClaimsPrincipal claimsPrincipal) + /// The from which to retrieve the user flow ID. + /// User Flow ID of the identity, or null if it cannot be found. + public static string? GetUserFlowId(this ClaimsPrincipal claimsPrincipal) { - string userFlowId = claimsPrincipal.FindFirstValue(ClaimConstants.Tfp); + if (claimsPrincipal == null) + { + throw new ArgumentNullException(nameof(claimsPrincipal)); + } + + string? userFlowId = claimsPrincipal.FindFirstValue(ClaimConstants.Tfp); if (string.IsNullOrEmpty(userFlowId)) { return claimsPrincipal.FindFirstValue(ClaimConstants.UserFlow); @@ -148,7 +173,7 @@ public static string GetUserFlowId(this ClaimsPrincipal claimsPrincipal) /// /// The from which to retrieve the sub claim. /// Home Object ID (sub) of the identity, or null if it cannot be found. - public static string GetHomeObjectId(this ClaimsPrincipal claimsPrincipal) + public static string? GetHomeObjectId(this ClaimsPrincipal claimsPrincipal) { if (claimsPrincipal == null) { @@ -163,7 +188,7 @@ public static string GetHomeObjectId(this ClaimsPrincipal claimsPrincipal) /// /// The from which to retrieve the sub claim. /// Home Tenant ID (sub) of the identity, or null if it cannot be found. - public static string GetHomeTenantId(this ClaimsPrincipal claimsPrincipal) + public static string? GetHomeTenantId(this ClaimsPrincipal claimsPrincipal) { if (claimsPrincipal == null) { @@ -178,8 +203,13 @@ public static string GetHomeTenantId(this ClaimsPrincipal claimsPrincipal) /// /// The from which to retrieve the uid claim. /// Name identifier ID (uid) of the identity, or null if it cannot be found. - public static string GetNameIdentifierId(this ClaimsPrincipal claimsPrincipal) + public static string? GetNameIdentifierId(this ClaimsPrincipal claimsPrincipal) { + if (claimsPrincipal == null) + { + throw new ArgumentNullException(nameof(claimsPrincipal)); + } + return claimsPrincipal.FindFirstValue(ClaimConstants.UniqueObjectIdentifier); } } diff --git a/src/Microsoft.Identity.Web/ClaimsPrincipalFactory.cs b/src/Microsoft.Identity.Web/ClaimsPrincipalFactory.cs index 2b02e0ea6..d591232d1 100644 --- a/src/Microsoft.Identity.Web/ClaimsPrincipalFactory.cs +++ b/src/Microsoft.Identity.Web/ClaimsPrincipalFactory.cs @@ -6,19 +6,19 @@ namespace Microsoft.Identity.Web { /// - /// Factory class to create ClaimsPrincipal objects. + /// Factory class to create objects. /// public static class ClaimsPrincipalFactory { /// - /// Instantiate a ClaimsPrincipal from an account objectId and tenantId. This can - /// be useful when the Web app subscribes to another service on behalf of the user - /// and then is called back by a notification where the user is identified by his tenant - /// id and object id (like in Microsoft Graph Web Hooks). + /// Instantiate a from an account object ID and tenant ID. This can + /// be useful when the web app subscribes to another service on behalf of the user + /// and then is called back by a notification where the user is identified by their tenant + /// ID and object ID (like in Microsoft Graph Web Hooks). /// - /// Tenant Id of the account. - /// Object Id of the account in this tenant ID. - /// A ClaimsPrincipal containing these two claims. + /// Tenant ID of the account. + /// Object ID of the account in this tenant ID. + /// A containing these two claims. /// /// /// @@ -43,4 +43,4 @@ public static ClaimsPrincipal FromTenantIdAndObjectId(string tenantId, string ob })); } } -} \ No newline at end of file +} diff --git a/src/Microsoft.Identity.Web/ClientInfo.cs b/src/Microsoft.Identity.Web/ClientInfo.cs index 502418cce..8cd99feee 100644 --- a/src/Microsoft.Identity.Web/ClientInfo.cs +++ b/src/Microsoft.Identity.Web/ClientInfo.cs @@ -2,7 +2,6 @@ // Licensed under the MIT License. using System; -using System.Runtime.Serialization; using System.Text.Json; using System.Text.Json.Serialization; @@ -11,12 +10,12 @@ namespace Microsoft.Identity.Web internal class ClientInfo { [JsonPropertyName("uid")] - public string UniqueObjectIdentifier { get; set; } + public string UniqueObjectIdentifier { get; set; } = null!; [JsonPropertyName("utid")] - public string UniqueTenantIdentifier { get; set; } + public string UniqueTenantIdentifier { get; set; } = null!; - public static ClientInfo CreateFromJson(string clientInfo) + public static ClientInfo? CreateFromJson(string clientInfo) { if (string.IsNullOrEmpty(clientInfo)) { @@ -26,7 +25,7 @@ public static ClientInfo CreateFromJson(string clientInfo) return DeserializeFromJson(Base64UrlHelpers.DecodeToBytes(clientInfo)); } - internal static ClientInfo DeserializeFromJson(byte[] jsonByteArray) + internal static ClientInfo? DeserializeFromJson(byte[] jsonByteArray) { if (jsonByteArray == null || jsonByteArray.Length == 0) { diff --git a/src/Microsoft.Identity.Web/CookiePolicyOptionsExtensions.cs b/src/Microsoft.Identity.Web/CookiePolicyOptionsExtensions.cs index f8dc8abfa..4862eac0d 100644 --- a/src/Microsoft.Identity.Web/CookiePolicyOptionsExtensions.cs +++ b/src/Microsoft.Identity.Web/CookiePolicyOptionsExtensions.cs @@ -16,8 +16,8 @@ public static class CookiePolicyOptionsExtensions /// Handles SameSite cookie issue according to the https://docs.microsoft.com/en-us/aspnet/core/security/samesite?view=aspnetcore-3.1. /// The default list of user-agents that disallow SameSite None, was taken from https://devblogs.microsoft.com/aspnet/upcoming-samesite-cookie-changes-in-asp-net-and-asp-net-core/. /// - /// - /// + /// to update. + /// to chain. public static CookiePolicyOptions HandleSameSiteCookieCompatibility(this CookiePolicyOptions options) { return HandleSameSiteCookieCompatibility(options, DisallowsSameSiteNone); @@ -27,16 +27,22 @@ public static CookiePolicyOptions HandleSameSiteCookieCompatibility(this CookieP /// Handles SameSite cookie issue according to the docs: https://docs.microsoft.com/en-us/aspnet/core/security/samesite?view=aspnetcore-3.1 /// The default list of user-agents that disallow SameSite None, was taken from https://devblogs.microsoft.com/aspnet/upcoming-samesite-cookie-changes-in-asp-net-and-asp-net-core/. /// - /// - /// If you dont want to use the default user-agent list implementation, the method sent in this parameter will be run against the user-agent and if returned true, SameSite value will be set to Unspecified. The default user-agent list used can be found at: https://devblogs.microsoft.com/aspnet/upcoming-samesite-cookie-changes-in-asp-net-and-asp-net-core/. - /// + /// to update. + /// If you don't want to use the default user-agent list implementation, the method sent in this parameter will be run against the user-agent and if returned true, SameSite value will be set to Unspecified. The default user-agent list used can be found at: https://devblogs.microsoft.com/aspnet/upcoming-samesite-cookie-changes-in-asp-net-and-asp-net-core/. + /// to chain. public static CookiePolicyOptions HandleSameSiteCookieCompatibility(this CookiePolicyOptions options, Func disallowsSameSiteNone) { + if (options == null) + { + throw new ArgumentNullException(nameof(options)); + } + options.MinimumSameSitePolicy = SameSiteMode.Unspecified; options.OnAppendCookie = cookieContext => CheckSameSite(cookieContext.Context, cookieContext.CookieOptions, disallowsSameSiteNone); options.OnDeleteCookie = cookieContext => CheckSameSite(cookieContext.Context, cookieContext.CookieOptions, disallowsSameSiteNone); + return options; } @@ -53,44 +59,47 @@ private static void CheckSameSite(HttpContext httpContext, CookieOptions options } /// - /// + /// Checks if the specified user agent supports SameSite None cookies. /// - /// + /// Browser user agent. /// Method taken from https://devblogs.microsoft.com/aspnet/upcoming-samesite-cookie-changes-in-asp-net-and-asp-net-core/. - /// + /// True, if the user agent does not allow SameSite None cookie; otherwise, false. public static bool DisallowsSameSiteNone(string userAgent) { - // Cover all iOS based browsers here. This includes: - // - Safari on iOS 12 for iPhone, iPod Touch, iPad - // - WkWebview on iOS 12 for iPhone, iPod Touch, iPad - // - Chrome on iOS 12 for iPhone, iPod Touch, iPad - // All of which are broken by SameSite=None, because they use the iOS networking - // stack. - if (userAgent.Contains("CPU iPhone OS 12") || - userAgent.Contains("iPad; CPU OS 12")) + if (!string.IsNullOrEmpty(userAgent)) { - return true; - } + // Cover all iOS based browsers here. This includes: + // - Safari on iOS 12 for iPhone, iPod Touch, iPad + // - WkWebview on iOS 12 for iPhone, iPod Touch, iPad + // - Chrome on iOS 12 for iPhone, iPod Touch, iPad + // All of which are broken by SameSite=None, because they use the iOS networking + // stack. + if (userAgent.Contains("CPU iPhone OS 12") || + userAgent.Contains("iPad; CPU OS 12")) + { + return true; + } - // Cover Mac OS X based browsers that use the Mac OS networking stack. - // This includes: - // - Safari on Mac OS X. - // This does not include: - // - Chrome on Mac OS X - // Because they do not use the Mac OS networking stack. - if (userAgent.Contains("Macintosh; Intel Mac OS X 10_14") && - userAgent.Contains("Version/") && userAgent.Contains("Safari")) - { - return true; - } + // Cover Mac OS X based browsers that use the Mac OS networking stack. + // This includes: + // - Safari on Mac OS X. + // This does not include: + // - Chrome on Mac OS X + // Because they do not use the Mac OS networking stack. + if (userAgent.Contains("Macintosh; Intel Mac OS X 10_14") && + userAgent.Contains("Version/") && userAgent.Contains("Safari")) + { + return true; + } - // Cover Chrome 50-69, because some versions are broken by SameSite=None, - // and none in this range require it. - // Note: this covers some pre-Chromium Edge versions, - // but pre-Chromium Edge does not require SameSite=None. - if (userAgent.Contains("Chrome/5") || userAgent.Contains("Chrome/6")) - { - return true; + // Cover Chrome 50-69, because some versions are broken by SameSite=None, + // and none in this range require it. + // Note: this covers some pre-Chromium Edge versions, + // but pre-Chromium Edge does not require SameSite=None. + if (userAgent.Contains("Chrome/5") || userAgent.Contains("Chrome/6")) + { + return true; + } } return false; diff --git a/src/Microsoft.Identity.Web/HttpContextExtensions.cs b/src/Microsoft.Identity.Web/HttpContextExtensions.cs index 86758add1..9c5741074 100644 --- a/src/Microsoft.Identity.Web/HttpContextExtensions.cs +++ b/src/Microsoft.Identity.Web/HttpContextExtensions.cs @@ -9,12 +9,12 @@ namespace Microsoft.Identity.Web internal static class HttpContextExtensions { /// - /// Keep the validated token associated with the Http request. + /// Keep the validated token associated with the HTTP request. /// - /// Http context. + /// HTTP context. /// Token to preserve after the token is validated so that /// it can be used in the actions. - internal static void StoreTokenUsedToCallWebAPI(this HttpContext httpContext, JwtSecurityToken token) + internal static void StoreTokenUsedToCallWebAPI(this HttpContext httpContext, JwtSecurityToken? token) { httpContext.Items.Add("JwtSecurityTokenUsedToCallWebAPI", token); } @@ -22,9 +22,9 @@ internal static void StoreTokenUsedToCallWebAPI(this HttpContext httpContext, Jw /// /// Get the parsed information about the token used to call the Web API. /// - /// Http context associated with the current request. - /// used to call the Web API. - internal static JwtSecurityToken GetTokenUsedToCallWebAPI(this HttpContext httpContext) + /// HTTP context associated with the current request. + /// used to call the web API. + internal static JwtSecurityToken? GetTokenUsedToCallWebAPI(this HttpContext httpContext) { return httpContext.Items["JwtSecurityTokenUsedToCallWebAPI"] as JwtSecurityToken; } diff --git a/src/Microsoft.Identity.Web/ITokenAcquisition.cs b/src/Microsoft.Identity.Web/ITokenAcquisition.cs index 4ec300350..9959eacab 100644 --- a/src/Microsoft.Identity.Web/ITokenAcquisition.cs +++ b/src/Microsoft.Identity.Web/ITokenAcquisition.cs @@ -22,7 +22,7 @@ public interface ITokenAcquisition /// 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. /// An access token to call on behalf of the user, the downstream API characterized by its scopes. - Task GetAccessTokenForUserAsync(IEnumerable scopes, string tenantId = null); + Task GetAccessTokenForUserAsync(IEnumerable scopes, string? tenantId = null); /// /// Acquires a token from the authority configured in the app, for the confidential client itself (not on behalf of a user) @@ -42,7 +42,7 @@ public interface ITokenAcquisition /// /// Scopes to consent to. /// triggering the challenge. - void ReplyForbiddenWithWwwAuthenticateHeader( + Task ReplyForbiddenWithWwwAuthenticateHeaderAsync( IEnumerable scopes, MsalUiRequiredException msalSeviceException); } diff --git a/src/Microsoft.Identity.Web/ITokenAcquisitionInternal.cs b/src/Microsoft.Identity.Web/ITokenAcquisitionInternal.cs index 658ea3f61..7c187bbb0 100644 --- a/src/Microsoft.Identity.Web/ITokenAcquisitionInternal.cs +++ b/src/Microsoft.Identity.Web/ITokenAcquisitionInternal.cs @@ -20,6 +20,7 @@ internal interface ITokenAcquisitionInternal /// /// The context used when an 'AuthorizationCode' is received over the OpenIdConnect protocol. /// Scopes to request. + /// A that represents a completed add to cache operation. /// /// From the configuration of the Authentication of the ASP.NET Core Web API: /// OpenIdConnectOptions options; @@ -47,7 +48,7 @@ internal interface ITokenAcquisitionInternal /// /// RedirectContext passed-in to a /// OpenID Connect event. - /// + /// A that represents a completed remove from cache operation. Task RemoveAccountAsync(RedirectContext context); } } diff --git a/src/Microsoft.Identity.Web/InstanceDiscovery/IssuerConfigurationRetriever.cs b/src/Microsoft.Identity.Web/InstanceDiscovery/IssuerConfigurationRetriever.cs index 0cd0dd26a..49430142e 100644 --- a/src/Microsoft.Identity.Web/InstanceDiscovery/IssuerConfigurationRetriever.cs +++ b/src/Microsoft.Identity.Web/InstanceDiscovery/IssuerConfigurationRetriever.cs @@ -10,7 +10,7 @@ namespace Microsoft.Identity.Web.InstanceDiscovery { /// - /// An implementation of IConfigurationRetriever geared towards Azure AD issuers metadata />. + /// An implementation of IConfigurationRetriever geared towards Azure AD issuers metadata. /// internal class IssuerConfigurationRetriever : IConfigurationRetriever { @@ -18,15 +18,17 @@ internal class IssuerConfigurationRetriever : IConfigurationRetrieverAddress of the discovery document. /// The to use to read the discovery document. /// A cancellation token that can be used by other objects or threads to receive notice of cancellation. . - /// - /// address - Azure AD Issuer metadata address url is required + /// + /// A that, when completed, returns from the configuration. + /// + /// address - Azure AD Issuer metadata address URL is required /// or /// retriever - No metadata document retriever is provided. public async Task GetConfigurationAsync(string address, IDocumentRetriever retriever, CancellationToken cancel) { if (string.IsNullOrEmpty(address)) { - throw new ArgumentNullException(nameof(address), $"Azure AD Issuer metadata address url is required"); + throw new ArgumentNullException(nameof(address), $"Azure AD Issuer metadata address URL is required"); } if (retriever == null) @@ -40,7 +42,7 @@ public async Task GetConfigurationAsync(string address, IDocumen }; string doc = await retriever.GetDocumentAsync(address, cancel).ConfigureAwait(false); - return JsonSerializer.Deserialize(doc, options); + return JsonSerializer.Deserialize(doc, options)!; // Note: The analyzer says Deserialize can return null, but the method comment says it just throws exceptions. } } } diff --git a/src/Microsoft.Identity.Web/InstanceDiscovery/IssuerMetadata.cs b/src/Microsoft.Identity.Web/InstanceDiscovery/IssuerMetadata.cs index 2e7e3763e..e4356b340 100644 --- a/src/Microsoft.Identity.Web/InstanceDiscovery/IssuerMetadata.cs +++ b/src/Microsoft.Identity.Web/InstanceDiscovery/IssuerMetadata.cs @@ -15,18 +15,18 @@ internal class IssuerMetadata /// Tenant discovery endpoint. /// [JsonPropertyName("tenant_discovery_endpoint")] - public string TenantDiscoveryEndpoint { get; set; } + public string? TenantDiscoveryEndpoint { get; set; } /// /// API Version. /// [JsonPropertyName("api-version")] - public string ApiVersion { get; set; } + public string? ApiVersion { get; set; } /// /// List of metadata associated with the endpoint. /// [JsonPropertyName("metadata")] - public List Metadata { get; set; } + public List Metadata { get; set; } = new List(); } } diff --git a/src/Microsoft.Identity.Web/InstanceDiscovery/Metadata.cs b/src/Microsoft.Identity.Web/InstanceDiscovery/Metadata.cs index 56f23fef4..561817e4d 100644 --- a/src/Microsoft.Identity.Web/InstanceDiscovery/Metadata.cs +++ b/src/Microsoft.Identity.Web/InstanceDiscovery/Metadata.cs @@ -15,19 +15,19 @@ internal class Metadata /// Preferred alias. /// [JsonPropertyName("preferred_network")] - public string PreferredNetwork { get; set; } + public string? PreferredNetwork { get; set; } /// /// Preferred alias to cache tokens emitted by one of the aliases (to avoid /// SSO islands). /// [JsonPropertyName("preferred_cache")] - public string PreferredCache { get; set; } + public string? PreferredCache { get; set; } /// /// Aliases of issuer URLs which are equivalent. /// [JsonPropertyName("aliases")] - public List Aliases { get; set; } + public List Aliases { get; set; } = new List(); } } diff --git a/src/Microsoft.Identity.Web/Microsoft.Identity.Web.csproj b/src/Microsoft.Identity.Web/Microsoft.Identity.Web.csproj index 86c53c856..0ff6b1079 100644 --- a/src/Microsoft.Identity.Web/Microsoft.Identity.Web.csproj +++ b/src/Microsoft.Identity.Web/Microsoft.Identity.Web.csproj @@ -47,12 +47,13 @@ - + - netcoreapp3.1; net5.0 + netcoreapp3.1; net5.0 true ../../build/MSAL.snk true + enable @@ -75,7 +76,7 @@ - + @@ -89,5 +90,5 @@ runtime; build; native; contentfiles; analyzers; buildtransitive - + diff --git a/src/Microsoft.Identity.Web/MicrosoftIdentityOptions.cs b/src/Microsoft.Identity.Web/MicrosoftIdentityOptions.cs index f7178dca5..588f9a1eb 100644 --- a/src/Microsoft.Identity.Web/MicrosoftIdentityOptions.cs +++ b/src/Microsoft.Identity.Web/MicrosoftIdentityOptions.cs @@ -16,17 +16,17 @@ public class MicrosoftIdentityOptions : OpenIdConnectOptions /// /// Gets or sets the Azure Active Directory instance, e.g. "https://login.microsoftonline.com". /// - public string Instance { get; set; } + public string Instance { get; set; } = null!; /// - /// Gets or sets the tenant Id. + /// Gets or sets the tenant ID. /// - public string TenantId { get; set; } + public string? TenantId { get; set; } /// /// Gets or sets the domain of the Azure Active Directory tenant, e.g. contoso.onmicrosoft.com. /// - public string Domain { get; set; } + public string? Domain { get; set; } /// /// Gets or sets TokenAcquisition as a Singleton. There are scenarios, like using the Graph SDK, @@ -37,22 +37,22 @@ public class MicrosoftIdentityOptions : OpenIdConnectOptions /// /// Gets or sets the edit profile user flow name for B2C, e.g. b2c_1_edit_profile. /// - public string EditProfilePolicyId { get; set; } + public string? EditProfilePolicyId { get; set; } /// /// Gets or sets the sign up or sign in user flow name for B2C, e.g. b2c_1_susi. /// - public string SignUpSignInPolicyId { get; set; } + public string? SignUpSignInPolicyId { get; set; } /// /// Gets or sets the reset password user flow name for B2C, e.g. B2C_1_password_reset. /// - public string ResetPasswordPolicyId { get; set; } + public string? ResetPasswordPolicyId { get; set; } /// /// Gets the default user flow (which is signUpsignIn). /// - public string DefaultUserFlow => SignUpSignInPolicyId; + public string? DefaultUserFlow => SignUpSignInPolicyId; /// /// Is considered B2C if the attribute SignUpSignInPolicyId is defined. @@ -78,7 +78,7 @@ internal bool IsB2C /// /// See also https://aka.ms/ms-id-web-certificates. /// - public IEnumerable ClientCertificates { get; set; } + public IEnumerable? ClientCertificates { get; set; } /// /// Description of the certificates used to decrypt an encrypted token in a Web API. @@ -96,7 +96,7 @@ internal bool IsB2C /// /// See also https://aka.ms/ms-id-web-certificates. /// - public IEnumerable TokenDecryptionCertificates { get; set; } + public IEnumerable? TokenDecryptionCertificates { get; set; } /// /// Specifies if the x5c claim (public key of the certificate) should be sent to the STS. diff --git a/src/Microsoft.Identity.Web/MicrosoftIdentityOptionsValidation.cs b/src/Microsoft.Identity.Web/MicrosoftIdentityOptionsValidation.cs index a54d91fab..5b14638b5 100644 --- a/src/Microsoft.Identity.Web/MicrosoftIdentityOptionsValidation.cs +++ b/src/Microsoft.Identity.Web/MicrosoftIdentityOptionsValidation.cs @@ -1,7 +1,6 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -using System.Collections; using System.Collections.Generic; using System.Globalization; using Microsoft.Extensions.Options; @@ -42,8 +41,8 @@ public ValidateOptionsResult Validate(string name, MicrosoftIdentityOptions opti } public static void ValidateEitherClientCertificateOrClientSecret( - string clientSecret, - IEnumerable cert) + string? clientSecret, + IEnumerable? cert) { if (string.IsNullOrEmpty(clientSecret) && (cert == null)) { diff --git a/src/Microsoft.Identity.Web/MigrationAid/Obsolete-LegacyTokenDecryptCertificateParameter.cs b/src/Microsoft.Identity.Web/MigrationAid/Obsolete-LegacyTokenDecryptCertificateParameter.cs index d87464edb..65be0d50a 100644 --- a/src/Microsoft.Identity.Web/MigrationAid/Obsolete-LegacyTokenDecryptCertificateParameter.cs +++ b/src/Microsoft.Identity.Web/MigrationAid/Obsolete-LegacyTokenDecryptCertificateParameter.cs @@ -13,7 +13,7 @@ namespace Microsoft.Identity.Web /// internal static class ObsoleteLegacyTokenDecryptCertificateParameter { - internal static void HandleLegacyTokenDecryptionCertificateParameter(MicrosoftIdentityOptions options, Action configureMicrosoftIdentityOptions, X509Certificate2 tokenDecryptionCertificate) + internal static void HandleLegacyTokenDecryptionCertificateParameter(MicrosoftIdentityOptions options, Action configureMicrosoftIdentityOptions, X509Certificate2? tokenDecryptionCertificate) { // Case where a legacy tokenDecryptionCertificate was passed. We then replace // the delegate called by the developer by a delegate which calls the delegate diff --git a/src/Microsoft.Identity.Web/MigrationAid/Obsolete-WebApiAuthenticationBuilderExtensions.cs b/src/Microsoft.Identity.Web/MigrationAid/Obsolete-WebApiAuthenticationBuilderExtensions.cs index bf2b013b4..693ad9623 100644 --- a/src/Microsoft.Identity.Web/MigrationAid/Obsolete-WebApiAuthenticationBuilderExtensions.cs +++ b/src/Microsoft.Identity.Web/MigrationAid/Obsolete-WebApiAuthenticationBuilderExtensions.cs @@ -33,12 +33,12 @@ public static AuthenticationBuilder AddProtectedWebApi( IConfiguration configuration, string configSectionName = "AzureAd", string jwtBearerScheme = JwtBearerDefaults.AuthenticationScheme, - X509Certificate2 tokenDecryptionCertificate = null, + X509Certificate2? tokenDecryptionCertificate = null, bool subscribeToJwtBearerMiddlewareDiagnosticsEvents = false) { // Just call the obsolete method below (which takes delegates). // This method will do the work of taking into account the legacy - // parameter for the token decyrption certificate + // parameter for the token decryption certificate return builder.AddProtectedWebApi( options => configuration.Bind(configSectionName, options), options => configuration.Bind(configSectionName, options), @@ -66,7 +66,7 @@ public static AuthenticationBuilder AddProtectedWebApi( this AuthenticationBuilder builder, Action configureJwtBearerOptions, Action configureMicrosoftIdentityOptions, - X509Certificate2 tokenDecryptionCertificate = null, + X509Certificate2? tokenDecryptionCertificate = null, string jwtBearerScheme = JwtBearerDefaults.AuthenticationScheme, bool subscribeToJwtBearerMiddlewareDiagnosticsEvents = false) { diff --git a/src/Microsoft.Identity.Web/MigrationAid/Obsolete-WebApiServiceCollectionExtensions.cs b/src/Microsoft.Identity.Web/MigrationAid/Obsolete-WebApiServiceCollectionExtensions.cs index 3ac3dac61..848ae82cb 100644 --- a/src/Microsoft.Identity.Web/MigrationAid/Obsolete-WebApiServiceCollectionExtensions.cs +++ b/src/Microsoft.Identity.Web/MigrationAid/Obsolete-WebApiServiceCollectionExtensions.cs @@ -35,7 +35,7 @@ public static IServiceCollection AddProtectedWebApi( IConfiguration configuration, string configSectionName = "AzureAd", string jwtBearerScheme = JwtBearerDefaults.AuthenticationScheme, - X509Certificate2 tokenDecryptionCertificate = null, + X509Certificate2? tokenDecryptionCertificate = null, bool subscribeToJwtBearerMiddlewareDiagnosticsEvents = false) { AuthenticationBuilder builder = services.AddAuthentication(jwtBearerScheme); @@ -66,7 +66,7 @@ public static IServiceCollection AddProtectedWebApi( this IServiceCollection services, Action configureJwtBearerOptions, Action configureMicrosoftIdentityOptions, - X509Certificate2 tokenDecryptionCertificate = null, + X509Certificate2? tokenDecryptionCertificate = null, string jwtBearerScheme = JwtBearerDefaults.AuthenticationScheme, bool subscribeToJwtBearerMiddlewareDiagnosticsEvents = false) { diff --git a/src/Microsoft.Identity.Web/MigrationAid/Obsolete-WebAppServiceCollectionExtensions.cs b/src/Microsoft.Identity.Web/MigrationAid/Obsolete-WebAppServiceCollectionExtensions.cs index 9fb9f016e..815cc1eac 100644 --- a/src/Microsoft.Identity.Web/MigrationAid/Obsolete-WebAppServiceCollectionExtensions.cs +++ b/src/Microsoft.Identity.Web/MigrationAid/Obsolete-WebAppServiceCollectionExtensions.cs @@ -69,7 +69,7 @@ public static IServiceCollection AddSignIn( /// /// Set to true if you want to debug, or just understand the OpenIdConnect events. /// - /// Yhe service collection for chaining. + /// The service collection for chaining. [Obsolete("Use AuthenticationBuilder.AddMicrosoftWebApp. See https://aka.ms/ms-id-web/net5")] public static IServiceCollection AddSignIn( this IServiceCollection services, diff --git a/src/Microsoft.Identity.Web/Resource/AadIssuerValidator.cs b/src/Microsoft.Identity.Web/Resource/AadIssuerValidator.cs index 938309eda..a33b2c75d 100644 --- a/src/Microsoft.Identity.Web/Resource/AadIssuerValidator.cs +++ b/src/Microsoft.Identity.Web/Resource/AadIssuerValidator.cs @@ -49,19 +49,19 @@ public static AadIssuerValidator GetIssuerValidator(string aadAuthority) throw new ArgumentNullException(nameof(aadAuthority)); } - Uri.TryCreate(aadAuthority, UriKind.Absolute, out Uri authorityUri); + Uri.TryCreate(aadAuthority, UriKind.Absolute, out Uri? authorityUri); string authorityHost = authorityUri?.Authority ?? new Uri(FallbackAuthority).Authority; - if (s_issuerValidators.TryGetValue(authorityHost, out AadIssuerValidator aadIssuerValidator)) + if (s_issuerValidators.TryGetValue(authorityHost, out AadIssuerValidator? aadIssuerValidator)) { return aadIssuerValidator; } // In the constructor, we hit the Azure AD issuer metadata endpoint and cache the aliases. The data is cached for 24 hrs. - var issuerMetadata = s_configManager.GetConfigurationAsync().ConfigureAwait(false).GetAwaiter().GetResult(); + IssuerMetadata issuerMetadata = s_configManager.GetConfigurationAsync().ConfigureAwait(false).GetAwaiter().GetResult(); // Add issuer aliases of the chosen authority to the cache - var aliases = issuerMetadata.Metadata + IEnumerable aliases = issuerMetadata.Metadata .Where(m => m.Aliases.Any(a => string.Equals(a, authorityHost, StringComparison.OrdinalIgnoreCase))) .SelectMany(m => m.Aliases) .Append(authorityHost) // For B2C scenarios, the alias will be the authority itself @@ -139,8 +139,8 @@ private bool IsValidIssuer(string validIssuerTemplate, string tenantId, string a try { - var issuerFromTemplateUri = new Uri(validIssuerTemplate.Replace("{tenantid}", tenantId)); - var actualIssuerUri = new Uri(actualIssuer); + Uri issuerFromTemplateUri = new Uri(validIssuerTemplate.Replace("{tenantid}", tenantId)); + Uri actualIssuerUri = new Uri(actualIssuer); // Template authority is in the aliases return _issuerAliases.Contains(issuerFromTemplateUri.Authority) && @@ -173,9 +173,9 @@ private static string GetTenantIdFromToken(SecurityToken securityToken) { if (securityToken is JwtSecurityToken jwtSecurityToken) { - if (jwtSecurityToken.Payload.TryGetValue(ClaimConstants.Tid, out object tenantId)) + if (jwtSecurityToken.Payload.TryGetValue(ClaimConstants.Tid, out object? tenantId)) { - return tenantId as string; + return (string)tenantId; } // Since B2C doesn't have "tid" as default, get it from issuer @@ -184,7 +184,7 @@ private static string GetTenantIdFromToken(SecurityToken securityToken) if (securityToken is JsonWebToken jsonWebToken) { - jsonWebToken.TryGetPayloadValue(ClaimConstants.Tid, out string tid); + jsonWebToken.TryGetPayloadValue(ClaimConstants.Tid, out string? tid); if (tid != null) { return tid; diff --git a/src/Microsoft.Identity.Web/Resource/IJwtBearerMiddlewareDiagnostics.cs b/src/Microsoft.Identity.Web/Resource/IJwtBearerMiddlewareDiagnostics.cs index aa8b16acc..24e3fde2a 100644 --- a/src/Microsoft.Identity.Web/Resource/IJwtBearerMiddlewareDiagnostics.cs +++ b/src/Microsoft.Identity.Web/Resource/IJwtBearerMiddlewareDiagnostics.cs @@ -6,15 +6,15 @@ namespace Microsoft.Identity.Web.Resource { /// - /// Interface implemented by diagnostics for the JwtBearer middleware. + /// Interface implemented by diagnostics for the JWT Bearer middleware. /// public interface IJwtBearerMiddlewareDiagnostics { /// - /// Called to subscribe to JwtBearerEvents. + /// Called to subscribe to . /// - /// JwtBearer events. - /// the events (for chaining). + /// JWT Bearer events. + /// The events (for chaining). JwtBearerEvents Subscribe(JwtBearerEvents events); } } diff --git a/src/Microsoft.Identity.Web/Resource/IOpenIdConnectMiddlewareDiagnostics.cs b/src/Microsoft.Identity.Web/Resource/IOpenIdConnectMiddlewareDiagnostics.cs index f17d7c7ba..dcaf98b16 100644 --- a/src/Microsoft.Identity.Web/Resource/IOpenIdConnectMiddlewareDiagnostics.cs +++ b/src/Microsoft.Identity.Web/Resource/IOpenIdConnectMiddlewareDiagnostics.cs @@ -6,15 +6,15 @@ namespace Microsoft.Identity.Web.Resource { /// - /// Diagnostics used in the Open Id Connect middleware + /// Diagnostics used in the OpenID Connect middleware /// (used in Web Apps). /// public interface IOpenIdConnectMiddlewareDiagnostics { /// - /// Method to subscribe to OpenIDConnect events. + /// Method to subscribe to . /// - /// Open Id connect events. + /// OpenID Connect events. void Subscribe(OpenIdConnectEvents events); } } diff --git a/src/Microsoft.Identity.Web/Resource/JwtBearerMiddlewareDiagnostics.cs b/src/Microsoft.Identity.Web/Resource/JwtBearerMiddlewareDiagnostics.cs index 9df29ce79..f502b3ccd 100644 --- a/src/Microsoft.Identity.Web/Resource/JwtBearerMiddlewareDiagnostics.cs +++ b/src/Microsoft.Identity.Web/Resource/JwtBearerMiddlewareDiagnostics.cs @@ -28,22 +28,22 @@ public JwtBearerMiddlewareDiagnostics(ILogger lo /// /// Invoked if exceptions are thrown during request processing. The exceptions will be re-thrown after this event unless suppressed. /// - private Func s_onAuthenticationFailed; + private Func s_onAuthenticationFailed = null!; /// /// Invoked when a protocol message is first received. /// - private Func s_onMessageReceived; + private Func s_onMessageReceived = null!; /// /// Invoked after the security token has passed validation and a ClaimsIdentity has been generated. /// - private Func s_onTokenValidated; + private Func s_onTokenValidated = null!; /// /// Invoked before a challenge is sent back to the caller. /// - private Func s_onChallenge; + private Func s_onChallenge = null!; /// /// Subscribes to all the JwtBearer events, to help debugging, while @@ -52,10 +52,7 @@ public JwtBearerMiddlewareDiagnostics(ILogger lo /// Events to subscribe to. public JwtBearerEvents Subscribe(JwtBearerEvents events) { - if (events == null) - { - events = new JwtBearerEvents(); - } + events ??= new JwtBearerEvents(); s_onAuthenticationFailed = events.OnAuthenticationFailed; events.OnAuthenticationFailed = OnAuthenticationFailedAsync; diff --git a/src/Microsoft.Identity.Web/Resource/OpenIdConnectMiddlewareDiagnostics.cs b/src/Microsoft.Identity.Web/Resource/OpenIdConnectMiddlewareDiagnostics.cs index e6a910e60..202d304ec 100644 --- a/src/Microsoft.Identity.Web/Resource/OpenIdConnectMiddlewareDiagnostics.cs +++ b/src/Microsoft.Identity.Web/Resource/OpenIdConnectMiddlewareDiagnostics.cs @@ -10,7 +10,7 @@ namespace Microsoft.Identity.Web.Resource { /// - /// Diagnostics used in the Open Id Connect middleware + /// Diagnostics used in the OpenID Connect middleware /// (used in Web Apps). /// public class OpenIdConnectMiddlewareDiagnostics : IOpenIdConnectMiddlewareDiagnostics @@ -32,46 +32,46 @@ public OpenIdConnectMiddlewareDiagnostics(ILogger s_onRedirectToIdentityProvider; + private Func s_onRedirectToIdentityProvider = null!; // Summary: // Invoked when a protocol message is first received. - private Func s_onMessageReceived; + private Func s_onMessageReceived = null!; // Summary: // Invoked after security token validation if an authorization code is present in // the protocol message. - private Func s_onAuthorizationCodeReceived; + private Func s_onAuthorizationCodeReceived = null!; // Summary: // Invoked after "authorization code" is redeemed for tokens at the token endpoint. - private Func s_onTokenResponseReceived; + private Func s_onTokenResponseReceived = null!; // Summary: // Invoked when an IdToken has been validated and produced an AuthenticationTicket. - private Func s_onTokenValidated; + private Func s_onTokenValidated = null!; // Summary: // Invoked when user information is retrieved from the UserInfoEndpoint. - private Func s_onUserInformationReceived; + private Func s_onUserInformationReceived = null!; // Summary: // Invoked if exceptions are thrown during request processing. The exceptions will // be re-thrown after this event unless suppressed. - private Func s_onAuthenticationFailed; + private Func s_onAuthenticationFailed = null!; // Summary: // Invoked when a request is received on the RemoteSignOutPath. - private Func s_onRemoteSignOut; + private Func s_onRemoteSignOut = null!; // Summary: // Invoked before redirecting to the identity provider to sign out. - private Func s_onRedirectToIdentityProviderForSignOut; + private Func s_onRedirectToIdentityProviderForSignOut = null!; // Summary: // Invoked before redirecting to the Microsoft.AspNetCore.Authentication.OpenIdConnect.OpenIdConnectOptions.SignedOutRedirectUri // at the end of a remote sign-out flow. - private Func s_onSignedOutCallbackRedirect; + private Func s_onSignedOutCallbackRedirect = null!; /// /// Subscribes to all the OpenIdConnect events, to help debugging, while @@ -80,6 +80,8 @@ public OpenIdConnectMiddlewareDiagnostics(ILoggerEvents to subscribe to. public void Subscribe(OpenIdConnectEvents events) { + events ??= new OpenIdConnectEvents(); + s_onRedirectToIdentityProvider = events.OnRedirectToIdentityProvider; events.OnRedirectToIdentityProvider = OnRedirectToIdentityProviderAsync; @@ -126,7 +128,7 @@ private void DisplayProtocolMessage(OpenIdConnectMessage message) { foreach (var property in message.GetType().GetProperties()) { - object value = property.GetValue(message); + object? value = property.GetValue(message); if (value != null) { _logger.LogDebug($" - {property.Name}={value}"); diff --git a/src/Microsoft.Identity.Web/Resource/RegisterValidAudience.cs b/src/Microsoft.Identity.Web/Resource/RegisterValidAudience.cs index d60301085..1cde375ef 100644 --- a/src/Microsoft.Identity.Web/Resource/RegisterValidAudience.cs +++ b/src/Microsoft.Identity.Web/Resource/RegisterValidAudience.cs @@ -14,7 +14,7 @@ namespace Microsoft.Identity.Web.Resource /// internal class RegisterValidAudience { - private string ClientId { get; set; } + private string ClientId { get; set; } = null!; private bool IsB2C { get; set; } = false; private const string Version = "ver"; private const string V1 = "1.0"; @@ -61,7 +61,12 @@ public void RegisterAudienceValidation( SecurityToken securityToken, TokenValidationParameters validationParameters) { - JwtSecurityToken token = securityToken as JwtSecurityToken; + JwtSecurityToken? token = securityToken as JwtSecurityToken; + if (token == null) + { + throw new SecurityTokenValidationException("Token is not JWT token."); + } + string validAudience; // Case of a default App ID URI (the developer did not provide explicit valid audience(s) diff --git a/src/Microsoft.Identity.Web/Resource/ScopesRequiredHttpContextExtensions.cs b/src/Microsoft.Identity.Web/Resource/ScopesRequiredHttpContextExtensions.cs index 8c27cbf3c..1a6da6e14 100644 --- a/src/Microsoft.Identity.Web/Resource/ScopesRequiredHttpContextExtensions.cs +++ b/src/Microsoft.Identity.Web/Resource/ScopesRequiredHttpContextExtensions.cs @@ -29,17 +29,22 @@ public static class ScopesRequiredHttpContextExtensions /// public static void VerifyUserHasAnyAcceptedScope(this HttpContext context, params string[] acceptedScopes) { + if (context == null) + { + throw new ArgumentNullException(nameof(context)); + } + if (acceptedScopes == null) { throw new ArgumentNullException(nameof(acceptedScopes)); } - Claim scopeClaim = context?.User?.FindFirst(ClaimConstants.Scope); + Claim? scopeClaim = context.User?.FindFirst(ClaimConstants.Scope); // Fallback to scp claim name if (scopeClaim == null) { - scopeClaim = context?.User?.FindFirst(ClaimConstants.Scp); + scopeClaim = context.User?.FindFirst(ClaimConstants.Scp); } if (scopeClaim == null || !scopeClaim.Value.Split(' ').Intersect(acceptedScopes).Any()) diff --git a/src/Microsoft.Identity.Web/ServiceCollectionExtensions.cs b/src/Microsoft.Identity.Web/ServiceCollectionExtensions.cs index ac25e68eb..4fe17ef95 100644 --- a/src/Microsoft.Identity.Web/ServiceCollectionExtensions.cs +++ b/src/Microsoft.Identity.Web/ServiceCollectionExtensions.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +using System; using Microsoft.Extensions.DependencyInjection; namespace Microsoft.Identity.Web @@ -33,6 +34,11 @@ public static IServiceCollection AddTokenAcquisition( this IServiceCollection services, bool isTokenAcquisitionSingleton = false) { + if (services == null) + { + throw new ArgumentNullException(nameof(services)); + } + // Token acquisition service services.AddHttpContextAccessor(); if (!isTokenAcquisitionSingleton) diff --git a/src/Microsoft.Identity.Web/TokenAcquisition.cs b/src/Microsoft.Identity.Web/TokenAcquisition.cs index af05852a0..c3b0de0af 100644 --- a/src/Microsoft.Identity.Web/TokenAcquisition.cs +++ b/src/Microsoft.Identity.Web/TokenAcquisition.cs @@ -33,10 +33,10 @@ internal class TokenAcquisition : ITokenAcquisition, ITokenAcquisitionInternal private readonly IMsalTokenCacheProvider _tokenCacheProvider; - private IConfidentialClientApplication _application; + private IConfidentialClientApplication? _application; private readonly IHttpContextAccessor _httpContextAccessor; private HttpContext CurrentHttpContext => _httpContextAccessor.HttpContext; - private IMsalHttpClientFactory _httpClientFactory; + private readonly IMsalHttpClientFactory _httpClientFactory; private readonly ILogger _logger; /// @@ -48,7 +48,7 @@ internal class TokenAcquisition : ITokenAcquisition, ITokenAcquisitionInternal /// Access to the HttpContext of the request. /// Configuration options. /// MSAL.NET configuration options. - /// Http client factory. + /// HTTP client factory. /// Logger. public TokenAcquisition( IMsalTokenCacheProvider tokenCacheProvider, @@ -131,7 +131,7 @@ public async Task AddAccountToCacheFromAuthorizationCodeAsync( // If they are not yet in the HttpContext.User's claims, add them here. if (!context.HttpContext.User.Claims.Any()) { - (context.HttpContext.User.Identity as ClaimsIdentity).AddClaims(context.Principal.Claims); + (context.HttpContext.User.Identity as ClaimsIdentity)?.AddClaims(context.Principal.Claims); } _application = await GetOrBuildConfidentialClientApplicationAsync().ConfigureAwait(false); @@ -177,7 +177,7 @@ public async Task AddAccountToCacheFromAuthorizationCodeAsync( [Obsolete("This method has been deprecated, please use the GetAccessTokenForUserAsync() method instead.")] public async Task GetAccessTokenOnBehalfOfUserAsync( IEnumerable scopes, - string tenant = null) + string? tenant = null) { return await GetAccessTokenForUserAsync(scopes, tenant).ConfigureAwait(false); } @@ -193,7 +193,7 @@ public async Task GetAccessTokenOnBehalfOfUserAsync( /// Scopes to request for the downstream API to call. /// Enables overriding of 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, like where the user is a guest in. - /// An access token to call the downstream API and populated with this downstream Api's scopes. + /// An access token to call the downstream API and populated with this downstream API's scopes. /// Calling this method from a Web API supposes that you have previously called, /// in a method called by JwtBearerOptions.Events.OnTokenValidated, the HttpContextExtensions.StoreTokenUsedToCallWebAPI method /// passing the validated token (as a JwtSecurityToken). Calling it from a Web App supposes that @@ -201,7 +201,7 @@ public async Task GetAccessTokenOnBehalfOfUserAsync( /// OpenIdConnectOptions.Events.OnAuthorizationCodeReceived. public async Task GetAccessTokenForUserAsync( IEnumerable scopes, - string tenant = null) + string? tenant = null) { if (scopes == null) { @@ -224,7 +224,7 @@ public async Task GetAccessTokenForUserAsync( // to get a token for a Web API on behalf of the user, but not necessarily with the on behalf of OAuth2.0 // flow as this one only applies to Web APIs. - JwtSecurityToken validatedToken = CurrentHttpContext.GetTokenUsedToCallWebAPI(); + JwtSecurityToken? validatedToken = CurrentHttpContext.GetTokenUsedToCallWebAPI(); // Case of Web APIs: we need to do an on-behalf-of flow if (validatedToken != null) @@ -285,12 +285,10 @@ public async Task GetAccessTokenForAppAsync(IEnumerable scopes) /// /// RedirectContext passed-in to a /// OpenID Connect event. - /// + /// A that represents a completed account removal operation. public async Task RemoveAccountAsync(RedirectContext context) { - ClaimsPrincipal user = context.HttpContext.User; IConfidentialClientApplication app = await GetOrBuildConfidentialClientApplicationAsync().ConfigureAwait(false); - IAccount account = null; // For B2C, we should remove all accounts of the user regardless the user flow if (_microsoftIdentityOptions.IsB2C) @@ -306,8 +304,8 @@ public async Task RemoveAccountAsync(RedirectContext context) } else { - string identifier = context.HttpContext.User.GetMsalAccountId(); - account = await app.GetAccountAsync(identifier).ConfigureAwait(false); + string? identifier = context.HttpContext.User.GetMsalAccountId(); + IAccount account = await app.GetAccountAsync(identifier).ConfigureAwait(false); if (account != null) { @@ -320,7 +318,6 @@ public async Task RemoveAccountAsync(RedirectContext context) /// /// Creates an MSAL Confidential client application if needed. /// - /// private async Task GetOrBuildConfidentialClientApplicationAsync() { if (_application == null) @@ -348,9 +345,6 @@ private async Task BuildConfidentialClientApplic _applicationOptions.Instance += "/"; } - string authority; - IConfidentialClientApplication app; - MicrosoftIdentityOptionsValidation.ValidateEitherClientCertificateOrClientSecret( _applicationOptions.ClientSecret, _microsoftIdentityOptions.ClientCertificates); @@ -362,6 +356,8 @@ private async Task BuildConfidentialClientApplic .WithRedirectUri(currentUri) .WithHttpClientFactory(_httpClientFactory); + string authority; + if (_microsoftIdentityOptions.IsB2C) { authority = $"{_applicationOptions.Instance}tfp/{_microsoftIdentityOptions.Domain}/{_microsoftIdentityOptions.DefaultUserFlow}"; @@ -375,11 +371,11 @@ private async Task BuildConfidentialClientApplic if (_microsoftIdentityOptions.ClientCertificates != null) { - X509Certificate2 certificate = DefaultCertificateLoader.LoadFirstCertificate(_microsoftIdentityOptions.ClientCertificates); + X509Certificate2? certificate = DefaultCertificateLoader.LoadFirstCertificate(_microsoftIdentityOptions.ClientCertificates); builder.WithCertificate(certificate); } - app = builder.Build(); + IConfidentialClientApplication app = builder.Build(); // Initialize token cache providers await _tokenCacheProvider.InitializeAsync(app.AppTokenCache).ConfigureAwait(false); await _tokenCacheProvider.InitializeAsync(app.UserTokenCache).ConfigureAwait(false); @@ -399,7 +395,7 @@ private async Task BuildConfidentialClientApplic /// /// Gets an access token for a downstream API on behalf of the user described by its claimsPrincipal. /// - /// + /// . /// Claims principal for the user on behalf of whom to get a token. /// Scopes for the downstream API to call. /// (optional) Specific tenant for which to acquire a token to access the scopes @@ -408,12 +404,12 @@ private async Task GetAccessTokenOnBehalfOfUserFromCacheAsync( IConfidentialClientApplication application, ClaimsPrincipal claimsPrincipal, IEnumerable scopes, - string tenant) + string? tenant) { // Gets MsalAccountId for AAD and B2C scenarios - string accountIdentifier = claimsPrincipal.GetMsalAccountId(); - string loginHint = claimsPrincipal.GetLoginHint(); - IAccount account = null; + string? accountIdentifier = claimsPrincipal.GetMsalAccountId(); + string? loginHint = claimsPrincipal.GetLoginHint(); + IAccount? account = null; if (accountIdentifier != null) { @@ -436,8 +432,8 @@ private async Task GetAccessTokenOnBehalfOfUserFromCacheAsync( // If it is B2C and could not get an account (most likely because there is no tid claims), try to get it by user flow if (_microsoftIdentityOptions.IsB2C && account == null) { - string currentUserFlow = claimsPrincipal.GetUserFlowId(); - account = GetAccountByUserFlow(await application.GetAccountsAsync().ConfigureAwait(false), currentUserFlow); + string? currentUserFlow = claimsPrincipal.GetUserFlowId(); + account = GetAccountByUserFlow(await application.GetAccountsAsync().ConfigureAwait(false), currentUserFlow!); } return await GetAccessTokenOnBehalfOfUserFromCacheAsync(application, account, scopes, tenant).ConfigureAwait(false); @@ -453,9 +449,9 @@ private async Task GetAccessTokenOnBehalfOfUserFromCacheAsync( /// private async Task GetAccessTokenOnBehalfOfUserFromCacheAsync( IConfidentialClientApplication application, - IAccount account, + IAccount? account, IEnumerable scopes, - string tenant) + string? tenant) { if (scopes == null) { @@ -508,8 +504,8 @@ private async Task GetAccessTokenOnBehalfOfUserFromCacheAsync( /// the client can trigger an interaction with the user so that the user consents to more scopes. /// /// Scopes to consent to. - /// triggering the challenge. - public void ReplyForbiddenWithWwwAuthenticateHeader(IEnumerable scopes, MsalUiRequiredException msalServiceException) + /// The that triggered the challenge. + public async Task ReplyForbiddenWithWwwAuthenticateHeaderAsync(IEnumerable scopes, MsalUiRequiredException msalServiceException) { // A user interaction is required, but we are in a Web API, and therefore, we need to report back to the client through a www-Authenticate header https://tools.ietf.org/html/rfc6750#section-3.1 string proposedAction = "consent"; @@ -521,6 +517,8 @@ public void ReplyForbiddenWithWwwAuthenticateHeader(IEnumerable scopes, } } + _application = await GetOrBuildConfidentialClientApplicationAsync().ConfigureAwait(false); + string consentUrl = $"{_application.Authority}/oauth2/v2.0/authorize?client_id={_applicationOptions.ClientId}" + $"&response_type=code&redirect_uri={_application.AppConfig.RedirectUri}" + $"&response_mode=query&scope=offline_access%20{string.Join("%20", scopes)}"; @@ -534,14 +532,12 @@ public void ReplyForbiddenWithWwwAuthenticateHeader(IEnumerable scopes, }; string parameterString = string.Join(", ", parameters.Select(p => $"{p.Key}=\"{p.Value}\"")); - string scheme = "Bearer"; - StringValues v = new StringValues($"{scheme} {parameterString}"); var httpResponse = CurrentHttpContext.Response; var headers = httpResponse.Headers; httpResponse.StatusCode = (int)HttpStatusCode.Forbidden; - headers[HeaderNames.WWWAuthenticate] = v; + headers[HeaderNames.WWWAuthenticate] = new StringValues($"Bearer {parameterString}"); } private static bool AcceptedTokenVersionMismatch(MsalUiRequiredException msalSeviceException) @@ -556,10 +552,10 @@ private static bool AcceptedTokenVersionMismatch(MsalUiRequiredException msalSev /// /// Gets an IAccount for the current B2C user flow in the user claims. /// - /// - /// - /// - private IAccount GetAccountByUserFlow(IEnumerable accounts, string userFlow) + /// A collection of accounts to search through. + /// A B2C user flow. + /// An with the specified user flow. + private IAccount? GetAccountByUserFlow(IEnumerable accounts, string userFlow) { foreach (var account in accounts) { diff --git a/src/Microsoft.Identity.Web/TokenCacheProviders/Distributed/DistributedTokenCacheAdapterExtension.cs b/src/Microsoft.Identity.Web/TokenCacheProviders/Distributed/DistributedTokenCacheAdapterExtension.cs index d65e5ed2a..1d7ab3614 100644 --- a/src/Microsoft.Identity.Web/TokenCacheProviders/Distributed/DistributedTokenCacheAdapterExtension.cs +++ b/src/Microsoft.Identity.Web/TokenCacheProviders/Distributed/DistributedTokenCacheAdapterExtension.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +using System; using Microsoft.Extensions.DependencyInjection; namespace Microsoft.Identity.Web.TokenCacheProviders.Distributed @@ -12,7 +13,7 @@ public static class DistributedTokenCacheAdapterExtension { /// Adds both the app and per-user in-memory token caches. /// The services collection to add to. - /// + /// A to chain. public static IServiceCollection AddDistributedTokenCaches( this IServiceCollection services) { @@ -23,9 +24,15 @@ public static IServiceCollection AddDistributedTokenCaches( /// Adds the in-memory based application token cache to the service collection. /// The services collection to add to. + /// A to chain. public static IServiceCollection AddDistributedAppTokenCache( this IServiceCollection services) { + if (services == null) + { + throw new ArgumentNullException(nameof(services)); + } + services.AddDistributedMemoryCache(); services.AddSingleton(); return services; @@ -33,13 +40,19 @@ public static IServiceCollection AddDistributedAppTokenCache( /// Adds the in-memory based per user token cache to the service collection. /// The services collection to add to. + /// A to chain. public static IServiceCollection AddDistributedUserTokenCache( this IServiceCollection services) { + if (services == null) + { + throw new ArgumentNullException(nameof(services)); + } + services.AddDistributedMemoryCache(); services.AddHttpContextAccessor(); services.AddSingleton(); return services; } } -} \ No newline at end of file +} diff --git a/src/Microsoft.Identity.Web/TokenCacheProviders/Distributed/MsalDistributedTokenCacheAdapter.cs b/src/Microsoft.Identity.Web/TokenCacheProviders/Distributed/MsalDistributedTokenCacheAdapter.cs index dfc149dbe..20196968f 100644 --- a/src/Microsoft.Identity.Web/TokenCacheProviders/Distributed/MsalDistributedTokenCacheAdapter.cs +++ b/src/Microsoft.Identity.Web/TokenCacheProviders/Distributed/MsalDistributedTokenCacheAdapter.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +using System; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.Caching.Distributed; @@ -38,6 +39,11 @@ public MsalDistributedTokenCacheAdapter( IOptions cacheOptions) : base(microsoftIdentityOptions, httpContextAccessor) { + if (cacheOptions == null) + { + throw new ArgumentNullException(nameof(cacheOptions)); + } + _distributedCache = memoryCache; _cacheOptions = cacheOptions.Value; } diff --git a/src/Microsoft.Identity.Web/TokenCacheProviders/IMsalTokenCacheProvider.cs b/src/Microsoft.Identity.Web/TokenCacheProviders/IMsalTokenCacheProvider.cs index 4d773dd8e..d78208b46 100644 --- a/src/Microsoft.Identity.Web/TokenCacheProviders/IMsalTokenCacheProvider.cs +++ b/src/Microsoft.Identity.Web/TokenCacheProviders/IMsalTokenCacheProvider.cs @@ -15,13 +15,13 @@ public interface IMsalTokenCacheProvider /// Initializes a token cache (which can be a user token cache or an app token cache). /// /// Token cache for which to initialize the serialization. - /// + /// A that represents a completed initialization operation. Task InitializeAsync(ITokenCache tokenCache); /// /// Clear the cache. /// - /// + /// A that represents a completed clear operation. Task ClearAsync(); } } diff --git a/src/Microsoft.Identity.Web/TokenCacheProviders/InMemory/InMemoryTokenCacheProviderExtension.cs b/src/Microsoft.Identity.Web/TokenCacheProviders/InMemory/InMemoryTokenCacheProviderExtension.cs index c1fb65ee9..6914d7bd4 100644 --- a/src/Microsoft.Identity.Web/TokenCacheProviders/InMemory/InMemoryTokenCacheProviderExtension.cs +++ b/src/Microsoft.Identity.Web/TokenCacheProviders/InMemory/InMemoryTokenCacheProviderExtension.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +using System; using Microsoft.Extensions.DependencyInjection; namespace Microsoft.Identity.Web.TokenCacheProviders.InMemory @@ -16,10 +17,15 @@ public static class InMemoryTokenCacheProviderExtension public static IServiceCollection AddInMemoryTokenCaches( this IServiceCollection services) { + if (services == null) + { + throw new ArgumentNullException(nameof(services)); + } + services.AddMemoryCache(); services.AddHttpContextAccessor(); services.AddSingleton(); return services; } } -} \ No newline at end of file +} diff --git a/src/Microsoft.Identity.Web/TokenCacheProviders/MsalAbstractTokenCacheProvider.cs b/src/Microsoft.Identity.Web/TokenCacheProviders/MsalAbstractTokenCacheProvider.cs index 9013447cf..24c094aef 100644 --- a/src/Microsoft.Identity.Web/TokenCacheProviders/MsalAbstractTokenCacheProvider.cs +++ b/src/Microsoft.Identity.Web/TokenCacheProviders/MsalAbstractTokenCacheProvider.cs @@ -10,7 +10,9 @@ namespace Microsoft.Identity.Web.TokenCacheProviders { - /// + /// + /// Token cache provider with default implementation. + /// /// public abstract class MsalAbstractTokenCacheProvider : IMsalTokenCacheProvider { @@ -39,7 +41,7 @@ protected MsalAbstractTokenCacheProvider(IOptions micr /// Initializes the token cache serialization. /// /// Token cache to serialize/deserialize. - /// + /// A that represents a completed initialization operation. public Task InitializeAsync(ITokenCache tokenCache) { if (tokenCache == null) @@ -57,7 +59,8 @@ public Task InitializeAsync(ITokenCache tokenCache) /// /// Cache key. /// - private string GetCacheKey(bool isAppTokenCache) + /// The cache key. + private string? GetCacheKey(bool isAppTokenCache) { if (isAppTokenCache) { @@ -68,7 +71,7 @@ private string GetCacheKey(bool isAppTokenCache) // In the case of Web Apps, the cache key is the user account Id, and the expectation is that AcquireTokenSilent // should return a token otherwise this might require a challenge. // In the case Web APIs, the token cache key is a hash of the access token used to call the Web API - JwtSecurityToken jwtSecurityToken = _httpContextAccessor.HttpContext.GetTokenUsedToCallWebAPI(); + JwtSecurityToken? jwtSecurityToken = _httpContextAccessor.HttpContext.GetTokenUsedToCallWebAPI(); return (jwtSecurityToken != null) ? jwtSecurityToken.RawSignature : _httpContextAccessor.HttpContext.User.GetMsalAccountId(); } @@ -86,7 +89,7 @@ private async Task OnAfterAccessAsync(TokenCacheNotificationArgs args) // if the access operation resulted in a cache update if (args.HasStateChanged) { - string cacheKey = GetCacheKey(args.IsApplicationCache); + string? cacheKey = GetCacheKey(args.IsApplicationCache); if (!string.IsNullOrWhiteSpace(cacheKey)) { await WriteCacheBytesAsync(cacheKey, args.TokenCache.SerializeMsalV3()).ConfigureAwait(false); @@ -96,7 +99,7 @@ private async Task OnAfterAccessAsync(TokenCacheNotificationArgs args) private async Task OnBeforeAccessAsync(TokenCacheNotificationArgs args) { - string cacheKey = GetCacheKey(args.IsApplicationCache); + string? cacheKey = GetCacheKey(args.IsApplicationCache); if (!string.IsNullOrEmpty(cacheKey)) { @@ -109,7 +112,7 @@ private async Task OnBeforeAccessAsync(TokenCacheNotificationArgs args) /// if you want to ensure that no concurrent write takes place, use this notification to place a lock on the entry. /// /// Token cache notification arguments. - /// + /// A that represents a completed operation. protected virtual Task OnBeforeWriteAsync(TokenCacheNotificationArgs args) { return Task.CompletedTask; @@ -118,10 +121,16 @@ protected virtual Task OnBeforeWriteAsync(TokenCacheNotificationArgs args) /// /// Clear the cache. /// + /// A that represents a completed clear operation. public async Task ClearAsync() { - // This is a user token cache - await RemoveKeyAsync(GetCacheKey(false)).ConfigureAwait(false); + string? cacheKey = GetCacheKey(false); + + if (!string.IsNullOrEmpty(cacheKey)) + { + // This is a user token cache + await RemoveKeyAsync(cacheKey).ConfigureAwait(false); + } // TODO: Clear the cookie session if any. Get inspiration from // https://github.com/Azure-Samples/active-directory-aspnetcore-webapp-openidconnect-v2/issues/240 @@ -132,7 +141,7 @@ public async Task ClearAsync() /// /// Cache key. /// Bytes to write. - /// + /// A that represents a completed write operation. protected abstract Task WriteCacheBytesAsync(string cacheKey, byte[] bytes); /// @@ -146,6 +155,7 @@ public async Task ClearAsync() /// Method to be implemented by concrete cache serializers to remove an entry from the cache. /// /// Cache key. + /// A that represents a completed remove key operation. protected abstract Task RemoveKeyAsync(string cacheKey); } } diff --git a/src/Microsoft.Identity.Web/TokenCacheProviders/Session/SessionTokenCacheProviderExtension.cs b/src/Microsoft.Identity.Web/TokenCacheProviders/Session/SessionTokenCacheProviderExtension.cs index cb3fdd391..54e132441 100644 --- a/src/Microsoft.Identity.Web/TokenCacheProviders/Session/SessionTokenCacheProviderExtension.cs +++ b/src/Microsoft.Identity.Web/TokenCacheProviders/Session/SessionTokenCacheProviderExtension.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +using System; using System.Linq; using Microsoft.AspNetCore.Builder; using Microsoft.Extensions.DependencyInjection; @@ -35,6 +36,11 @@ public static class SessionTokenCacheProviderExtension /// The service collection. public static IServiceCollection AddSessionTokenCaches(this IServiceCollection services) { + if (services == null) + { + throw new ArgumentNullException(nameof(services)); + } + // Add session if you are planning to use session based token cache var sessionStoreService = services.FirstOrDefault(x => x.ServiceType.Name == "ISessionStore"); @@ -84,6 +90,11 @@ public static IServiceCollection AddSessionTokenCaches(this IServiceCollection s /// The service collection. public static IServiceCollection AddSessionAppTokenCache(this IServiceCollection services) { + if (services == null) + { + throw new ArgumentNullException(nameof(services)); + } + services.AddHttpContextAccessor(); services.AddScoped(); return services; @@ -112,6 +123,11 @@ public static IServiceCollection AddSessionAppTokenCache(this IServiceCollection /// The service collection. public static IServiceCollection AddSessionPerUserTokenCache(this IServiceCollection services) { + if (services == null) + { + throw new ArgumentNullException(nameof(services)); + } + services.AddHttpContextAccessor(); services.AddSession(option => { diff --git a/src/Microsoft.Identity.Web/WebApiAuthenticationBuilderExtensions.cs b/src/Microsoft.Identity.Web/WebApiAuthenticationBuilderExtensions.cs index 97de6a89e..a5f800356 100644 --- a/src/Microsoft.Identity.Web/WebApiAuthenticationBuilderExtensions.cs +++ b/src/Microsoft.Identity.Web/WebApiAuthenticationBuilderExtensions.cs @@ -72,7 +72,7 @@ public static AuthenticationBuilder AddMicrosoftWebApi( } builder.Services.Configure(jwtBearerScheme, configureJwtBearerOptions); - builder.Services.Configure(configureMicrosoftIdentityOptions); + builder.Services.Configure(configureMicrosoftIdentityOptions); builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton, MicrosoftIdentityOptionsValidation>()); builder.Services.AddHttpContextAccessor(); @@ -84,7 +84,7 @@ public static AuthenticationBuilder AddMicrosoftWebApi( { // TODO: // Suspect. Why not get the IOption? - var microsoftIdentityOptions = new MicrosoftIdentityOptions(); // configuration.GetSection(configSectionName).Get(); + MicrosoftIdentityOptions microsoftIdentityOptions = new MicrosoftIdentityOptions(); // configuration.GetSection(configSectionName).Get(); configureMicrosoftIdentityOptions(microsoftIdentityOptions); if (string.IsNullOrWhiteSpace(options.Authority)) diff --git a/src/Microsoft.Identity.Web/WebApiCallsWebApiAuthenticationBuilderExtensions.cs b/src/Microsoft.Identity.Web/WebApiCallsWebApiAuthenticationBuilderExtensions.cs index 318880bd7..ed91c7426 100644 --- a/src/Microsoft.Identity.Web/WebApiCallsWebApiAuthenticationBuilderExtensions.cs +++ b/src/Microsoft.Identity.Web/WebApiCallsWebApiAuthenticationBuilderExtensions.cs @@ -57,10 +57,20 @@ public static AuthenticationBuilder AddMicrosoftWebApiCallsWebApi( throw new ArgumentNullException(nameof(builder)); } + if (configureConfidentialClientApplicationOptions == null) + { + throw new ArgumentNullException(nameof(configureConfidentialClientApplicationOptions)); + } + + if (configureMicrosoftIdentityOptions == null) + { + throw new ArgumentNullException(nameof(configureMicrosoftIdentityOptions)); + } + builder.Services.Configure(configureConfidentialClientApplicationOptions); builder.Services.Configure(configureMicrosoftIdentityOptions); - var microsoftIdentityOptions = new MicrosoftIdentityOptions(); + MicrosoftIdentityOptions microsoftIdentityOptions = new MicrosoftIdentityOptions(); configureMicrosoftIdentityOptions(microsoftIdentityOptions); builder.Services.AddTokenAcquisition(microsoftIdentityOptions.SingletonTokenAcquisition); diff --git a/src/Microsoft.Identity.Web/WebAppAuthenticationBuilderExtensions.cs b/src/Microsoft.Identity.Web/WebAppAuthenticationBuilderExtensions.cs index 32d0d0ea1..07837edbc 100644 --- a/src/Microsoft.Identity.Web/WebAppAuthenticationBuilderExtensions.cs +++ b/src/Microsoft.Identity.Web/WebAppAuthenticationBuilderExtensions.cs @@ -67,11 +67,26 @@ public static AuthenticationBuilder AddMicrosoftWebApp( string cookieScheme = CookieAuthenticationDefaults.AuthenticationScheme, bool subscribeToOpenIdConnectMiddlewareDiagnosticsEvents = false) { + if (builder == null) + { + throw new ArgumentNullException(nameof(builder)); + } + + if (configureOpenIdConnectOptions == null) + { + throw new ArgumentNullException(nameof(configureOpenIdConnectOptions)); + } + + if (configureMicrosoftIdentityOptions == null) + { + throw new ArgumentNullException(nameof(configureMicrosoftIdentityOptions)); + } + builder.Services.Configure(openIdConnectScheme, configureOpenIdConnectOptions); builder.Services.Configure(configureMicrosoftIdentityOptions); builder.Services.AddHttpClient(); - var microsoftIdentityOptions = new MicrosoftIdentityOptions(); + MicrosoftIdentityOptions microsoftIdentityOptions = new MicrosoftIdentityOptions(); configureMicrosoftIdentityOptions(microsoftIdentityOptions); builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton, MicrosoftIdentityOptionsValidation>()); diff --git a/src/Microsoft.Identity.Web/WebAppCallsWebApiAuthenticationBuilderExtensions.cs b/src/Microsoft.Identity.Web/WebAppCallsWebApiAuthenticationBuilderExtensions.cs index 83d8c6550..83ee0da88 100644 --- a/src/Microsoft.Identity.Web/WebAppCallsWebApiAuthenticationBuilderExtensions.cs +++ b/src/Microsoft.Identity.Web/WebAppCallsWebApiAuthenticationBuilderExtensions.cs @@ -85,20 +85,35 @@ public static AuthenticationBuilder AddMicrosoftWebAppCallsWebApi( /// The authentication builder for chaining. public static AuthenticationBuilder AddMicrosoftWebAppCallsWebApi( this AuthenticationBuilder builder, - IEnumerable initialScopes, + IEnumerable? initialScopes, Action configureMicrosoftIdentityOptions, Action configureConfidentialClientApplicationOptions, string openIdConnectScheme = OpenIdConnectDefaults.AuthenticationScheme) { + if (builder == null) + { + throw new ArgumentNullException(nameof(builder)); + } + + if (configureMicrosoftIdentityOptions == null) + { + throw new ArgumentNullException(nameof(configureMicrosoftIdentityOptions)); + } + + if (configureConfidentialClientApplicationOptions == null) + { + throw new ArgumentNullException(nameof(configureConfidentialClientApplicationOptions)); + } + IServiceCollection services = builder.Services; // Ensure that configuration options for MSAL.NET, HttpContext accessor and the Token acquisition service // (encapsulating MSAL.NET) are available through dependency injection - services.Configure(configureMicrosoftIdentityOptions); - services.Configure(configureConfidentialClientApplicationOptions); + services.Configure(configureMicrosoftIdentityOptions); + services.Configure(configureConfidentialClientApplicationOptions); services.AddHttpContextAccessor(); - var microsoftIdentityOptions = new MicrosoftIdentityOptions(); + MicrosoftIdentityOptions microsoftIdentityOptions = new MicrosoftIdentityOptions(); configureMicrosoftIdentityOptions(microsoftIdentityOptions); services.AddTokenAcquisition(microsoftIdentityOptions.SingletonTokenAcquisition); @@ -126,7 +141,7 @@ public static AuthenticationBuilder AddMicrosoftWebAppCallsWebApi( var codeReceivedHandler = options.Events.OnAuthorizationCodeReceived; options.Events.OnAuthorizationCodeReceived = async context => { - var tokenAcquisition = context.HttpContext.RequestServices.GetRequiredService() as ITokenAcquisitionInternal; + var tokenAcquisition = (ITokenAcquisitionInternal)context.HttpContext.RequestServices.GetRequiredService(); await tokenAcquisition.AddAccountToCacheFromAuthorizationCodeAsync(context, options.Scope).ConfigureAwait(false); await codeReceivedHandler(context).ConfigureAwait(false); }; @@ -135,19 +150,18 @@ public static AuthenticationBuilder AddMicrosoftWebAppCallsWebApi( var onTokenValidatedHandler = options.Events.OnTokenValidated; options.Events.OnTokenValidated = async context => { - ClientInfo clientInfoFromServer; if (context.Request.Form.ContainsKey(ClaimConstants.ClientInfo)) { context.Request.Form.TryGetValue(ClaimConstants.ClientInfo, out Microsoft.Extensions.Primitives.StringValues value); if (!string.IsNullOrEmpty(value)) { - clientInfoFromServer = ClientInfo.CreateFromJson(value); + ClientInfo? clientInfoFromServer = ClientInfo.CreateFromJson(value); if (clientInfoFromServer != null) { - context.Principal.Identities.FirstOrDefault().AddClaim(new Claim(ClaimConstants.UniqueTenantIdentifier, clientInfoFromServer.UniqueTenantIdentifier)); - context.Principal.Identities.FirstOrDefault().AddClaim(new Claim(ClaimConstants.UniqueObjectIdentifier, clientInfoFromServer.UniqueObjectIdentifier)); + context.Principal.Identities.FirstOrDefault()?.AddClaim(new Claim(ClaimConstants.UniqueTenantIdentifier, clientInfoFromServer.UniqueTenantIdentifier)); + context.Principal.Identities.FirstOrDefault()?.AddClaim(new Claim(ClaimConstants.UniqueObjectIdentifier, clientInfoFromServer.UniqueObjectIdentifier)); } } } @@ -160,7 +174,7 @@ public static AuthenticationBuilder AddMicrosoftWebAppCallsWebApi( options.Events.OnRedirectToIdentityProviderForSignOut = async context => { // Remove the account from MSAL.NET token cache - var tokenAcquisition = context.HttpContext.RequestServices.GetRequiredService() as ITokenAcquisitionInternal; + var tokenAcquisition = (ITokenAcquisitionInternal)context.HttpContext.RequestServices.GetRequiredService(); await tokenAcquisition.RemoveAccountAsync(context).ConfigureAwait(false); await signOutHandler(context).ConfigureAwait(false); }; diff --git a/tests/.editorconfig b/tests/.editorconfig index 6a0a5d86d..075789f3f 100644 --- a/tests/.editorconfig +++ b/tests/.editorconfig @@ -5,6 +5,9 @@ # Code files [*.{cs,vb}] +# SA0001: XML comment analysis disabled +dotnet_diagnostic.SA0001.severity = none + # SA1600: Elements should be documented dotnet_diagnostic.SA1600.severity = none @@ -31,3 +34,6 @@ dotnet_diagnostic.SA1402.severity = none # SA1611: Element parameters should be documented dotnet_diagnostic.SA1611.severity = none + +# SA1604: Element documentation should have summary +dotnet_diagnostic.SA1604.severity = none diff --git a/tests/Microsoft.Identity.Web.Test/AccountExtensionsTests.cs b/tests/Microsoft.Identity.Web.Test/AccountExtensionsTests.cs index 1734495e8..c779c11d4 100644 --- a/tests/Microsoft.Identity.Web.Test/AccountExtensionsTests.cs +++ b/tests/Microsoft.Identity.Web.Test/AccountExtensionsTests.cs @@ -17,9 +17,7 @@ public void ToClaimsPrincipal_NullAccount_ReturnsNull() { IAccount account = null; - var claimsPrincipalResult = account.ToClaimsPrincipal(); - - Assert.Null(claimsPrincipalResult); + Assert.Throws("account", () => account.ToClaimsPrincipal()); } [Fact] @@ -34,6 +32,7 @@ public void ToClaimsPrincipal_ValidAccount_ReturnsClaimsPrincipal() account.HomeAccountId.Returns(new AccountId("identifier", oid, tid)); var claimsIdentityResult = account.ToClaimsPrincipal().Identity as ClaimsIdentity; + Assert.NotNull(claimsIdentityResult); Assert.Equal(3, claimsIdentityResult.Claims.Count()); Assert.Equal(username, claimsIdentityResult.FindFirst(ClaimTypes.Upn)?.Value); @@ -42,11 +41,14 @@ public void ToClaimsPrincipal_ValidAccount_ReturnsClaimsPrincipal() } [Fact] - public void ToClaimsPrincipal_AccountWithNullValues_ThrowsException() + public void ToClaimsPrincipal_AccountWithNullValues_ReturnsEmptyPrincipal() { IAccount account = Substitute.For(); - Assert.Throws("value", () => account.ToClaimsPrincipal()); + var claimsIdentityResult = account.ToClaimsPrincipal().Identity as ClaimsIdentity; + + Assert.NotNull(claimsIdentityResult); + Assert.Single(claimsIdentityResult.Claims); } } } diff --git a/tests/Microsoft.Identity.Web.Test/InstanceDiscovery/IssuerConfigurationRetrieverTests.cs b/tests/Microsoft.Identity.Web.Test/InstanceDiscovery/IssuerConfigurationRetrieverTests.cs index 54e0c3271..f000ee111 100644 --- a/tests/Microsoft.Identity.Web.Test/InstanceDiscovery/IssuerConfigurationRetrieverTests.cs +++ b/tests/Microsoft.Identity.Web.Test/InstanceDiscovery/IssuerConfigurationRetrieverTests.cs @@ -20,7 +20,7 @@ public class IssuerConfigurationRetrieverTests public async Task GetConfigurationAsync_NullOrEmptyParameters_ThrowsException() { var configurationRetriever = new IssuerConfigurationRetriever(); - var expectedAddressExceptionMessage = "Azure AD Issuer metadata address url is required (Parameter 'address')"; + var expectedAddressExceptionMessage = "Azure AD Issuer metadata address URL is required (Parameter 'address')"; var expectedRetrieverExceptionMessage = "No metadata document retriever is provided (Parameter 'retriever')"; var exception = await Assert.ThrowsAsync("address", () => configurationRetriever.GetConfigurationAsync(null, null, CancellationToken.None)).ConfigureAwait(false); diff --git a/tests/Microsoft.Identity.Web.Test/Resource/ScopesRequiredHttpContextExtensionsTests.cs b/tests/Microsoft.Identity.Web.Test/Resource/ScopesRequiredHttpContextExtensionsTests.cs index b0ea3c294..f3431a6b6 100644 --- a/tests/Microsoft.Identity.Web.Test/Resource/ScopesRequiredHttpContextExtensionsTests.cs +++ b/tests/Microsoft.Identity.Web.Test/Resource/ScopesRequiredHttpContextExtensionsTests.cs @@ -18,7 +18,7 @@ public void VerifyUserHasAnyAcceptedScope_NullParameters_ThrowsException() { HttpContext httpContext = null; - Assert.Throws(() => httpContext.VerifyUserHasAnyAcceptedScope(string.Empty)); + Assert.Throws("context", () => httpContext.VerifyUserHasAnyAcceptedScope(string.Empty)); httpContext = HttpContextUtilities.CreateHttpContext();