Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Nullable partial work #183

Closed
wants to merge 1 commit into from
Closed

Nullable partial work #183

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Microsoft.Identity.Web/AccountExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public static class AccountExtensions
/// </summary>
/// <param name="account">The IAccount instance.</param>
/// <returns>A <see cref="ClaimsPrincipal"/> built from IAccount.</returns>
public static ClaimsPrincipal ToClaimsPrincipal(this IAccount account)
public static ClaimsPrincipal? ToClaimsPrincipal(this IAccount? account)
Copy link
Member Author

Choose a reason for hiding this comment

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

Public API changes are backwards compatible.

Copy link
Contributor

Choose a reason for hiding this comment

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

Here and in similar methods, we can also use attributes like `[return: NotNullIfNotNull("account")]

{
if (account != null)
{
Expand Down
76 changes: 38 additions & 38 deletions src/Microsoft.Identity.Web/AuthorizeForScopesAttribute.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,64 +32,60 @@ public class AuthorizeForScopesAttribute : ExceptionFilterAttribute
/// <summary>
/// Scopes to request.
/// </summary>
public string[] Scopes { get; set; }
public string[]? Scopes { get; set; }

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

/// <summary>
/// Handles the MsalUiRequiredException.
/// </summary>
/// <param name="context">Context provided by ASP.NET Core.</param>
public override void OnException(ExceptionContext context)
Copy link
Member Author

Choose a reason for hiding this comment

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

I rewrote this method a bit, but all changes are non-breaking (done with analyzers)

{
// Do not re-use the attribute param Scopes. For more info: https://github.com/Azure-Samples/active-directory-aspnetcore-webapp-openidconnect-v2/issues/273
string[] incrementalConsentScopes = Array.Empty<string>();
MsalUiRequiredException msalUiRequiredException = context.Exception as MsalUiRequiredException;
MsalUiRequiredException? msalUiRequiredException =
(context.Exception as MsalUiRequiredException) ??
(context.Exception?.InnerException as MsalUiRequiredException);

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

if (msalUiRequiredException != null)
{
if (CanBeSolvedByReSignInOfUser(msalUiRequiredException))
// 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))
{
// the users cannot provide both scopes and ScopeKeySection at the same time
if (!string.IsNullOrWhiteSpace(ScopeKeySection) && Scopes != null && Scopes.Length > 0)
{
throw new InvalidOperationException($"Either provide the '{nameof(ScopeKeySection)}' or the '{nameof(Scopes)}' to the 'AuthorizeForScopes'.");
}
// Load the injected IConfiguration
IConfiguration configuration = context.HttpContext.RequestServices.GetRequiredService<IConfiguration>();

// If the user wishes us to pick the Scopes from a particular config setting.
if (!string.IsNullOrWhiteSpace(ScopeKeySection))
if (configuration == null)
{
// Load the injected IConfiguration
IConfiguration configuration = context.HttpContext.RequestServices.GetRequiredService<IConfiguration>();

if (configuration == null)
{
throw new InvalidOperationException($"The {nameof(ScopeKeySection)} is provided but the IConfiguration instance is not present in the services collection");
}
throw new InvalidOperationException($"The {nameof(ScopeKeySection)} is provided but the IConfiguration instance is not present in the services collection");
}

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

if (Scopes != null && Scopes.Length > 0 && incrementalConsentScopes != null && incrementalConsentScopes.Length > 0)
{
throw new InvalidOperationException("no scopes provided in scopes...");
}
}
else
if (Scopes != null && Scopes.Length > 0 && incrementalConsentScopes.Length > 0)
{
incrementalConsentScopes = Scopes;
throw new InvalidOperationException("no scopes provided in scopes...");
}

var properties = BuildAuthenticationPropertiesForIncrementalConsent(incrementalConsentScopes, msalUiRequiredException, context.HttpContext);
context.Result = new ChallengeResult(properties);
}
else
{
incrementalConsentScopes = Scopes;
}

var properties = BuildAuthenticationPropertiesForIncrementalConsent(incrementalConsentScopes, msalUiRequiredException, context.HttpContext);
context.Result = new ChallengeResult(properties);
}

base.OnException(context);
Expand All @@ -114,7 +110,7 @@ private bool CanBeSolvedByReSignInOfUser(MsalUiRequiredException ex)
/// <param name="context">current HTTP context in the pipeline.</param>
/// <returns>AuthenticationProperties.</returns>
private AuthenticationProperties BuildAuthenticationPropertiesForIncrementalConsent(
string[] scopes,
string[]? scopes,
MsalUiRequiredException ex,
HttpContext context)
{
Expand All @@ -128,9 +124,13 @@ private AuthenticationProperties BuildAuthenticationPropertiesForIncrementalCons
OidcConstants.ScopeProfile,
};

// TODO: scopes can actually be null here - how do we treat this case?
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the kind of bugs that can be discovered. In this particular case I am not sure how to handle it - please advise.

Copy link
Collaborator

Choose a reason for hiding this comment

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

yes...good point. This is for incremental consent, so we technically should only be here if we have scopes, because that's the point, that additional scopes are required. So I think in OnException(), we should make sure we have something. @jmprieur thoughts?

In reply to: 433215274 [](ancestors = 433215274)

// if this is not allowed then we should throw before calling this method
// if this is allowed, we need to avoid the null ref below

properties.SetParameter<ICollection<string>>(
OpenIdConnectParameterNames.Scope,
scopes.Union(additionalBuiltInScopes).ToList());
scopes.Union(additionalBuiltInScopes).ToList()); // potential null ref exception

// Attempts to set the login_hint to avoid the logged-in user to be presented with an account selection dialog
var loginHint = context.User.GetLoginHint();
Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.Identity.Web/Base64UrlHelpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ internal static class Base64UrlHelpers
// * the 62nd and 63rd regular base64 encoding characters ('+' and '/') are replace with ('-' and '_')
// The changes make the encoding alphabet file and URL safe
// See RFC4648, section 5 for more info
public static string Encode(string arg)
public static string? Encode(string? arg)
{
if (arg == null)
{
Expand Down Expand Up @@ -68,7 +68,7 @@ public static byte[] DecodeToBytes(string arg)
return Convert.FromBase64String(s); // Standard base64 decoder
}

internal static string Encode(byte[] arg)
internal static string? Encode(byte[]? arg)
{
if (arg == null)
{
Expand Down
8 changes: 4 additions & 4 deletions src/Microsoft.Identity.Web/ClaimsPrincipalExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public static class ClaimsPrincipalExtensions
/// </summary>
/// <param name="claimsPrincipal">Claims principal.</param>
/// <returns>A string corresponding to an account identifier as defined in <see cref="Microsoft.Identity.Client.AccountId.Identifier"/>.</returns>
public static string GetMsalAccountId(this ClaimsPrincipal claimsPrincipal)
public static string? GetMsalAccountId(this ClaimsPrincipal claimsPrincipal)
Copy link
Member Author

Choose a reason for hiding this comment

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

If a consuming application also enforces the "nullable reference" feature, then they will get warnings when they try to pass in a NULL, for example a null ClaimsPrincinpal here.

If the consuming application does not enforce this feature, they will not get warnings.

Copy link
Contributor

Choose a reason for hiding this comment

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

How do we decide if we should change the parameter to nullable or keep it non-nullable and throw the ArgumentNullException?

In this case, a null claimsPrincipal can return a null ID value and non-null claimsPrincipal returns non-null ID value.

Copy link
Member Author

Choose a reason for hiding this comment

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

At line 23 we throw an ArgumentNullEx if the claimsPrincipal is null, we don't return null.

{
if (claimsPrincipal == null)
Copy link
Member Author

Choose a reason for hiding this comment

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

From a code perspective, we still need to make null checks and program defensively, because NULLs can still be passed everywhere.

{
Expand Down Expand Up @@ -85,13 +85,13 @@ public static string GetLoginHint(this ClaimsPrincipal claimsPrincipal)
/// </summary>
/// <param name="claimsPrincipal">Identity for which to compute the domain-hint.</param>
/// <returns>domain-hint for the identity, or <c>null</c> if it cannot be found.</returns>
public static string GetDomainHint(this ClaimsPrincipal claimsPrincipal)
public static string? GetDomainHint(this ClaimsPrincipal 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";

Expand Down Expand Up @@ -154,7 +154,7 @@ public static string GetHomeObjectId(this ClaimsPrincipal claimsPrincipal)
{
throw new ArgumentNullException(nameof(claimsPrincipal));
}

return claimsPrincipal.FindFirstValue(ClaimConstants.UniqueObjectIdentifier);
}

Expand Down
20 changes: 6 additions & 14 deletions src/Microsoft.Identity.Web/ClientInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ namespace Microsoft.Identity.Web
internal class ClientInfo
{
[DataMember(Name = "uid", IsRequired = false)]
public string UniqueObjectIdentifier { get; set; }
public string? UniqueObjectIdentifier { get; set; }

[DataMember(Name = "utid", IsRequired = false)]
public string UniqueTenantIdentifier { get; set; }
public string? UniqueTenantIdentifier { get; set; }

public static ClientInfo CreateFromJson(string clientInfo)
{
Expand All @@ -25,19 +25,11 @@ public static ClientInfo CreateFromJson(string clientInfo)
throw new ArgumentNullException(nameof(clientInfo), $"client info returned from the server is null");
}

return DeserializeFromJson<ClientInfo>(Base64UrlHelpers.DecodeToBytes(clientInfo));
}

internal static T DeserializeFromJson<T>(byte[] jsonByteArray)
Copy link
Contributor

Choose a reason for hiding this comment

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

I would keep DeserializeFromJson. I think you can also leave the generic parameter and add where T : notnull.

{
if (jsonByteArray == null || jsonByteArray.Length == 0)
{
return default;
}
var jsonByteArray = Base64UrlHelpers.DecodeToBytes(clientInfo);

using var stream = new MemoryStream(jsonByteArray);
using var reader = new StreamReader(stream, Encoding.UTF8);
return (T)JsonSerializer.Create().Deserialize(reader, typeof(T));
using MemoryStream stream = new MemoryStream(jsonByteArray);
using StreamReader reader = new StreamReader(stream, Encoding.UTF8);
return (ClientInfo)JsonSerializer.Create().Deserialize(reader, typeof(ClientInfo));
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure I understand why you replaced the generic here (T) by ClientInfo?

Copy link
Member Author

Choose a reason for hiding this comment

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

JSON deserialization can always return NULL (since you don't know what you deserialize). But a nullable value type is not the same as a nullable reference type. So general purpose serialization helpers that just take a "T" won't work. I think @pmaytak 's suggestions (where T : notnull) might do it, I didn't research this enough.

Copy link
Member Author

Choose a reason for hiding this comment

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

But from a code perspective, why have a general purpose deserialization method in a class that only deserializes ClientInfo? Makes the code hard to read...

}
}
}
2 changes: 1 addition & 1 deletion src/Microsoft.Identity.Web/HttpContextExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ internal static void StoreTokenUsedToCallWebAPI(this HttpContext httpContext, Jw
/// </summary>
/// <param name="httpContext">Http context associated with the current request.</param>
/// <returns><see cref="JwtSecurityToken"/> used to call the Web API.</returns>
internal static JwtSecurityToken GetTokenUsedToCallWebAPI(this HttpContext httpContext)
internal static JwtSecurityToken? GetTokenUsedToCallWebAPI(this HttpContext httpContext)
Copy link
Contributor

Choose a reason for hiding this comment

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

In this method (and similar ones), we expect httpContext to have a value here. However, if null is passed in, we get an NPE. So we either need to return null or throw.

Copy link
Member Author

Choose a reason for hiding this comment

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

For public APIs, we should always check for NULLs and throw NullArgumentException.
For internal APIs like this - use your best judgement - I think in most cases we can add a null check, but this is where you know best. If you can't decide, my advice would be throw, i.e. be stingy with returning NULLs.

Copy link
Contributor

@pmaytak pmaytak Jun 3, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

i.e. be stingy with returning NULLs.

👍

{
return httpContext.Items["JwtSecurityTokenUsedToCallWebAPI"] as JwtSecurityToken;
}
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.Identity.Web/ITokenAcquisition.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public interface ITokenAcquisition
/// <param name="tenantId">Enables to override the tenant/account for the same identity. This is useful in the
/// cases where a given account is guest in other tenants, and you want to acquire tokens for a specific tenant.</param>
/// <returns>An access token to call on behalf of the user, the downstream API characterized by its scopes.</returns>
Task<string> GetAccessTokenForUserAsync(IEnumerable<string> scopes, string tenantId = null);
Task<string> GetAccessTokenForUserAsync(IEnumerable<string> scopes, string? tenantId = null);

/// <summary>
/// Acquires a token from the authority configured in the app, for the confidential client itself (not on behalf of a user)
Expand Down
8 changes: 4 additions & 4 deletions src/Microsoft.Identity.Web/InstanceDiscovery/IssuerMetadata.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,18 @@ internal class IssuerMetadata
/// Tenant discovery endpoint.
/// </summary>
[JsonProperty(PropertyName = "tenant_discovery_endpoint")]
public string TenantDiscoveryEndpoint { get; set; }
public string? TenantDiscoveryEndpoint { get; set; }

/// <summary>
/// API Version.
/// </summary>
[JsonProperty(PropertyName = "api-version")]
public string ApiVersion { get; set; }
public string? ApiVersion { get; set; }

/// <summary>
/// List of metadata associated with the endpoint.
/// </summary>
[JsonProperty(PropertyName = "metadata")]
public List<Metadata> Metadata { get; set; }
public List<Metadata>? Metadata { get; set; }
}
}
}
8 changes: 4 additions & 4 deletions src/Microsoft.Identity.Web/InstanceDiscovery/Metadata.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,19 @@ internal class Metadata
/// Preferred alias.
/// </summary>
[JsonProperty(PropertyName = "preferred_network")]
public string PreferredNetwork { get; set; }
public string? PreferredNetwork { get; set; }
Copy link
Contributor

Choose a reason for hiding this comment

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

This case is similar to Late-initialized properties, Data Transfer Objects and nullability. What is our intent - value is required for all these properties or optional?

Copy link
Member Author

Choose a reason for hiding this comment

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

Very good point, I missed this one!

Copy link
Contributor

Choose a reason for hiding this comment

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

Good thing about this work and PR is that it starts the discussions about the intent of our code. 👍


/// <summary>
/// Preferred alias to cache tokens emitted by one of the aliases (to avoid
/// SSO islands).
/// </summary>
[JsonProperty(PropertyName = "preferred_cache")]
public string PreferredCache { get; set; }
public string? PreferredCache { get; set; }

/// <summary>
/// Aliases of issuer URLs which are equivalent.
/// </summary>
[JsonProperty(PropertyName = "aliases")]
public List<string> Aliases { get; set; }
public List<string>? Aliases { get; set; }
}
}
}
31 changes: 10 additions & 21 deletions src/Microsoft.Identity.Web/Microsoft.Identity.Web.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>

<PropertyGroup>
<TargetFramework>netcoreapp3.1</TargetFramework>
<SignAssembly>true</SignAssembly>
<AssemblyOriginatorKeyFile>../../build/MSAL.snk</AssemblyOriginatorKeyFile>
<GenerateDocumentationFile>true</GenerateDocumentationFile>
<Nullable>enable</Nullable>
Copy link
Member Author

Choose a reason for hiding this comment

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

This is how the feature is enabled. It needs C# 8, which is available with .net core 3.1 or with .net class 4.smth (4.7.2?)

</PropertyGroup>

<PropertyGroup Label="Package Metadat">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Metadat? or Metadata?

<!--This should be passed from the VSTS build-->
<ClientSemVer Condition="'$(ClientSemVer)' == ''">0.1.0-localbuild</ClientSemVer>
<!--This will generate AssemblyVersion, AssemblyFileVersion and AssemblyInformationVersion-->
Expand All @@ -24,6 +32,7 @@
<PackageReleaseNotes>The changelog is available at https://github.com/AzureAD/microsoft-identity-web/blob/master/changelog.txt and the roadmap at https://github.com/AzureAD/microsoft-identity-web/wiki#roadmap </PackageReleaseNotes>
<PackageTags>Microsoft Identity Web .NET ASP.NET Core Web App Web API B2C</PackageTags>
</PropertyGroup>

<PropertyGroup Label="Source Link">
<PublishRepositoryUrl>true</PublishRepositoryUrl>
<EmbedUntrackedSources>true</EmbedUntrackedSources>
Expand All @@ -34,9 +43,6 @@

<ItemGroup>
<AdditionalFiles Include="..\..\stylecop.json" />
</ItemGroup>

<ItemGroup>
<None Include="..\..\LICENSE">
<Pack>True</Pack>
<PackagePath></PackagePath>
Expand All @@ -46,23 +52,6 @@
<ItemGroup Label="Build Tools" Condition="$([MSBuild]::IsOsPlatform('Windows'))">
<PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.0.0" PrivateAssets="All" />
</ItemGroup>


<PropertyGroup>
<TargetFramework>netcoreapp3.1</TargetFramework>
<SignAssembly>true</SignAssembly>
<AssemblyOriginatorKeyFile>../../build/MSAL.snk</AssemblyOriginatorKeyFile>
<GenerateDocumentationFile>true</GenerateDocumentationFile>
</PropertyGroup>

<PropertyGroup>
<!-- The MSAL.snk has both private and public keys -->
<DelaySign>false</DelaySign>
</PropertyGroup>

<PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|AnyCPU'">
<DocumentationFile>C:\gh\microsoft-identity-web\src\Microsoft.Identity.Web\Microsoft.Identity.Web.xml</DocumentationFile>
</PropertyGroup>

<ItemGroup>
<PackageReference Include="Microsoft.AspNetCore.Authentication.JwtBearer" Version="3.1.1" />
Expand Down
8 changes: 4 additions & 4 deletions src/Microsoft.Identity.Web/Resource/AadIssuerValidator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,10 @@ 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;
}
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.Identity.Web/Resource/OpenIdConnectMiddlewareDiagnostics.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,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}");
Expand Down
7 changes: 6 additions & 1 deletion src/Microsoft.Identity.Web/Resource/RegisterValidAudience.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,12 @@ public void RegisterAudienceValidation(
SecurityToken securityToken,
TokenValidationParameters validationParameters)
{
JwtSecurityToken token = securityToken as JwtSecurityToken;
JwtSecurityToken? token = securityToken as JwtSecurityToken;
if (token == null)
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the kind of bugs you discover when you turn this feature on. THe compiler does NULL analysis and points out areas of the code that are potentially unsafe.

{
throw new SecurityTokenValidationException("Token is not JWT token.");
bgavrilMS marked this conversation as resolved.
Show resolved Hide resolved
}

string validAudience;

// Case of a default App ID URI (the developer did not provide explicit valid audience(s)
Expand Down
Loading