Skip to content

Commit

Permalink
Cherry pick 2680 to 6x (#2687)
Browse files Browse the repository at this point in the history 
* Remove dependency on AadIssuerValidator.GetTenantIdFromToken in ValidateIssuerSigningKey (#2680)

* Remove dep on AadIssuerValidator
* Add DontFailOnMissingTidValidateIssuerSigning AppContextSwitch
* Fail if multiple tids

---------

Co-authored-by: Keegan Caruso <[email protected]>

* Update langver for tests

---------

Co-authored-by: Keegan Caruso <[email protected]>
  • Loading branch information
@keegan-caruso
keegan-caruso and Keegan Caruso authored Jul 9, 2024
1 parent d193668 commit 0d5b3bf
Show file tree
Hide file tree
Showing 6 changed files with 206 additions and 5 deletions.
2 changes: 1 addition & 1 deletion build/commonTest.props
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
<WarningsNotAsErrors>0618</WarningsNotAsErrors>
<IsTestProject>true</IsTestProject>
<IsPackable>false</IsPackable>
<LangVersion>11</LangVersion>
<LangVersion>12</LangVersion>
<TargetFrameworks Condition=" '$(OS)' == 'Windows_NT' ">$(TestTargets)</TargetFrameworks>
<TargetFrameworks Condition=" '$(OS)' != 'Windows_NT' ">$(TestOnlyCoreTargets)</TargetFrameworks>
<RuntimeFrameworkVersion Condition=" '$(TargetFramework)' == 'netcoreapp2.1'">$(DotNetCoreAppRuntimeVersion)</RuntimeFrameworkVersion>
Expand Down
4 changes: 4 additions & 0 deletions src/Microsoft.IdentityModel.JsonWebTokens/InternalsVisibleTo.cs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

[assembly: System.Runtime.CompilerServices.InternalsVisibleTo("Microsoft.IdentityModel.Validators, PublicKey=0024000004800000940000000602000000240000525341310004000001000100b5fc90e7027f67871e773a8fde8938c81dd402ba65b9201d60593e96c492651e889cc13f1415ebb53fac1131ae0bd333c5ee6021672d9718ea31a8aebd0da0072f25d87dba6fc90ffd598ed4da35e44c398c454307e8e33b8426143daec9f596836f97c8f74750e5975c64e2189f45def46b2a2b1247adc3652bf5c308055da9")]
20 changes: 20 additions & 0 deletions src/Microsoft.IdentityModel.JsonWebTokens/JsonWebToken.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,26 @@ internal byte[] CipherTextBytes
set;
}

/// <summary>
/// Gets the names of the payload claims on the JsonWebToken.
/// </summary>
internal IReadOnlyCollection<string> PayloadClaimNames
{
#if NET45
get
{
var payloadClaimNames = new List<string>();

foreach (Claim claim in Claims)
payloadClaimNames.Add(claim.Type);

return payloadClaimNames;
}
#else
get => Payload.Elements.Keys;
#endif
}

internal ClaimsIdentity ClaimsIdentity
{
get
Expand Down
67 changes: 64 additions & 3 deletions src/Microsoft.IdentityModel.Validators/AadTokenValidationParametersExtension.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@
// Licensed under the MIT License.

using System;
using System.Collections.Generic;
using System.IdentityModel.Tokens.Jwt;
using System.Linq;
using Microsoft.IdentityModel.JsonWebTokens;
using Microsoft.IdentityModel.Logging;
using Microsoft.IdentityModel.Protocols.OpenIdConnect;
using Microsoft.IdentityModel.Tokens;
Expand Down Expand Up @@ -41,6 +44,15 @@ public static void EnableAadSigningKeyIssuerValidation(this TokenValidationParam
};
}

#if !NET45
internal const string DontFailOnMissingTidSwitch = "Switch.Microsoft.IdentityModel.DontFailOnMissingTidValidateIssuerSigning";

private static bool DontFailOnMissingTid()
{
return (AppContext.TryGetSwitch(DontFailOnMissingTidSwitch, out bool dontFailOnMissingTid) && dontFailOnMissingTid);
}
#endif

/// <summary>
/// Validates the issuer signing key.
/// </summary>
Expand All @@ -67,9 +79,16 @@ internal static bool ValidateIssuerSigningKey(SecurityKey securityKey, SecurityT
if (string.IsNullOrWhiteSpace(signingKeyIssuer))
return true;

var tenantIdFromToken = AadIssuerValidator.GetTenantIdFromToken(securityToken);
var tenantIdFromToken = GetTid(securityToken);
if (string.IsNullOrEmpty(tenantIdFromToken))
return true;
{
#if !NET45
if (DontFailOnMissingTid())
return true;
#endif

throw LogHelper.LogExceptionMessage(new SecurityTokenInvalidIssuerException(LogMessages.IDX40009));
}

var tokenIssuer = securityToken.Issuer;

Expand All @@ -91,7 +110,7 @@ internal static bool ValidateIssuerSigningKey(SecurityKey securityKey, SecurityT

// comparing effectiveSigningKeyIssuer with v2TokenIssuer is required as well because of the following scenario:
// 1. service trusts /common/v2.0 endpoint
// 2. service receieves a v1 token that has issuer like sts.windows.net
// 2. service receives a v1 token that has issuer like sts.windows.net
// 3. signing key issuers will never match sts.windows.net as v1 endpoint doesn't have issuers attached to keys
// v2TokenIssuer is the representation of Token.Issuer (if it was a v2 issuer)
if (effectiveSigningKeyIssuer != tokenIssuer && effectiveSigningKeyIssuer != v2TokenIssuer)
Expand All @@ -101,6 +120,48 @@ internal static bool ValidateIssuerSigningKey(SecurityKey securityKey, SecurityT
return true;
}

private static string GetTid(SecurityToken securityToken)
{
switch (securityToken)
{
case JsonWebToken jsonWebToken:
if (jsonWebToken.TryGetPayloadValue<string>(AadIssuerValidatorConstants.Tid, out string tid))
{
EnforceSingleClaimCaseInsensitive(jsonWebToken.PayloadClaimNames, AadIssuerValidatorConstants.Tid);
return tid;
}

return string.Empty;

case JwtSecurityToken jwtSecurityToken:
if ((jwtSecurityToken.Payload.TryGetValue(AadIssuerValidatorConstants.Tid, out object tidObject) && tidObject is string jwtTid))
{
EnforceSingleClaimCaseInsensitive(jwtSecurityToken.Payload.Keys, AadIssuerValidatorConstants.Tid);
return jwtTid;
}

return string.Empty;

default:
throw LogHelper.LogExceptionMessage(new SecurityTokenInvalidIssuerException(LogMessages.IDX40010));
}
}

private static void EnforceSingleClaimCaseInsensitive(IEnumerable<string> keys, string claimType)
{
bool claimSeen = false;
foreach (var key in keys)
{
if (string.Equals(key, claimType, StringComparison.OrdinalIgnoreCase))
{
if (claimSeen)
throw LogHelper.LogExceptionMessage(new SecurityTokenInvalidIssuerException(LogHelper.FormatInvariant(LogMessages.IDX40011, claimType)));

claimSeen = true;
}
}
}

/// <summary>
/// Validates the issuer signing key certificate.
/// </summary>
Expand Down
4 changes: 4 additions & 0 deletions src/Microsoft.IdentityModel.Validators/LogMessages.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,9 @@ internal static class LogMessages
public const string IDX40004 = "IDX40004: Token issuer: '{0}', does not contain the `tid` or `tenantId` claim present in the token: '{1}'.";
public const string IDX40005 = "IDX40005: Token issuer: '{0}', does not match the signing key issuer: '{1}'.";
public const string IDX40007 = "IDX40007: RequireSignedTokens property on ValidationParameters is set to true, but the issuer signing key is null.";

public const string IDX40009 = "IDX40009: Either the 'tid' claim was not found or it didn't have a value.";
public const string IDX40010 = "IDX40010: The SecurityToken must be a 'JsonWebToken' or 'JwtSecurityToken'";
public const string IDX40011 = "IDX40011: The SecurityToken has multiple instances of the '{0}' claim.";
}
}
114 changes: 113 additions & 1 deletion test/Microsoft.IdentityModel.Validators.Tests/AadSigningKeyIssuerValidatorTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,15 @@
using Microsoft.IdentityModel.Protocols.OpenIdConnect;
using Microsoft.IdentityModel.TestUtils;
using Microsoft.IdentityModel.Tokens;
using Microsoft.IdentityModel.Tokens.Saml2;
using Xunit;

#pragma warning disable CS3016 // Arrays as attribute arguments is not CLS-compliant

namespace Microsoft.IdentityModel.Validators.Tests
{
// Serialize as one of the tests depends on static state (app context)
[Collection(nameof(AadSigningKeyIssuerValidatorTests))]
public class AadSigningKeyIssuerValidatorTests
{
[Theory, MemberData(nameof(EnableAadSigningKeyIssuerValidationTestCases))]
Expand Down Expand Up @@ -167,6 +170,7 @@ public void ValidateIssuerSigningKeyTests(AadSigningKeyIssuerTheoryData theoryDa

try
{
theoryData.SetupAction?.Invoke();
var result = AadTokenValidationParametersExtension.ValidateIssuerSigningKey(theoryData.SecurityKey, theoryData.SecurityToken, theoryData.OpenIdConnectConfiguration);
theoryData.ExpectedException.ProcessNoException(context);
Assert.True(result);
Expand All @@ -175,6 +179,10 @@ public void ValidateIssuerSigningKeyTests(AadSigningKeyIssuerTheoryData theoryDa
{
theoryData.ExpectedException.ProcessException(ex, context);
}
finally
{
theoryData.TearDownAction?.Invoke();
}

TestUtilities.AssertFailIfErrors(context);
}
Expand Down Expand Up @@ -294,7 +302,17 @@ public static TheoryData<AadSigningKeyIssuerTheoryData> ValidateIssuerSigningKey
TestId = "MissingTenantIdClaimInToken",
SecurityKey = KeyingMaterial.JsonWebKeyP256,
SecurityToken = new JwtSecurityToken(),
OpenIdConnectConfiguration = mockConfiguration
OpenIdConnectConfiguration = mockConfiguration,
ExpectedException = ExpectedException.SecurityTokenInvalidIssuerException("IDX40009")
});

theoryData.Add(new AadSigningKeyIssuerTheoryData
{
TestId = "WrongSecurityKeyType",
SecurityKey = KeyingMaterial.JsonWebKeyP256,
SecurityToken = new Saml2SecurityToken(new Saml2Assertion(new Saml2NameIdentifier("nameIdentifier"))),
OpenIdConnectConfiguration = mockConfiguration,
ExpectedException = ExpectedException.SecurityTokenInvalidIssuerException("IDX40010")
});

theoryData.Add(new AadSigningKeyIssuerTheoryData
Expand All @@ -321,6 +339,96 @@ public static TheoryData<AadSigningKeyIssuerTheoryData> ValidateIssuerSigningKey
ExpectedException = ExpectedException.SecurityTokenInvalidIssuerException("IDX40004")
});

#if !NET452
theoryData.Add(new AadSigningKeyIssuerTheoryData
{
TestId = "Doesnt_Fail_With_Switch",
SecurityKey = KeyingMaterial.JsonWebKeyP256,
SecurityToken = new JwtSecurityToken(),
OpenIdConnectConfiguration = mockConfiguration,
SetupAction = () => AppContext.SetSwitch(AadTokenValidationParametersExtension.DontFailOnMissingTidSwitch, true),
TearDownAction = () => AppContext.SetSwitch(AadTokenValidationParametersExtension.DontFailOnMissingTidSwitch, false)
});

theoryData.Add(new AadSigningKeyIssuerTheoryData
{
TestId = "Fail_With_Switch_False",
SecurityKey = KeyingMaterial.JsonWebKeyP256,
SecurityToken = new JwtSecurityToken(),
OpenIdConnectConfiguration = mockConfiguration,
ExpectedException = ExpectedException.SecurityTokenInvalidIssuerException("IDX40009"),
SetupAction = () => AppContext.SetSwitch(AadTokenValidationParametersExtension.DontFailOnMissingTidSwitch, false),
TearDownAction = () => AppContext.SetSwitch(AadTokenValidationParametersExtension.DontFailOnMissingTidSwitch, isEnabled: false)
});

theoryData.Add(new AadSigningKeyIssuerTheoryData
{
TestId = "Doesnt_Fail_With_Switch",
SecurityKey = KeyingMaterial.JsonWebKeyP256,
SecurityToken = new JwtSecurityToken(),
OpenIdConnectConfiguration = mockConfiguration,
SetupAction = () => AppContext.SetSwitch(AadTokenValidationParametersExtension.DontFailOnMissingTidSwitch, true),
TearDownAction = () => AppContext.SetSwitch(AadTokenValidationParametersExtension.DontFailOnMissingTidSwitch, false)
});

theoryData.Add(new AadSigningKeyIssuerTheoryData
{
TestId = "Fail_With_Switch_False_JsonWebToken",
SecurityKey = KeyingMaterial.JsonWebKeyP256,
SecurityToken = new JsonWebToken(Default.Jwt(Default.SecurityTokenDescriptor(Default.SymmetricSigningCredentials, [issClaim]))),
OpenIdConnectConfiguration = mockConfiguration,
ExpectedException = ExpectedException.SecurityTokenInvalidIssuerException("IDX40009"),
SetupAction = () => AppContext.SetSwitch(AadTokenValidationParametersExtension.DontFailOnMissingTidSwitch, false),
TearDownAction = () => AppContext.SetSwitch(AadTokenValidationParametersExtension.DontFailOnMissingTidSwitch, isEnabled: false)
});

theoryData.Add(new AadSigningKeyIssuerTheoryData
{
TestId = "Doesnt_Fail_With_Switch_JsonWebToken",
SecurityKey = KeyingMaterial.JsonWebKeyP256,
SecurityToken = new JsonWebToken(Default.Jwt(Default.SecurityTokenDescriptor(Default.SymmetricSigningCredentials, [issClaim]))),
OpenIdConnectConfiguration = mockConfiguration,
SetupAction = () => AppContext.SetSwitch(AadTokenValidationParametersExtension.DontFailOnMissingTidSwitch, true),
TearDownAction = () => AppContext.SetSwitch(AadTokenValidationParametersExtension.DontFailOnMissingTidSwitch, false)
});
#endif

theoryData.Add(new AadSigningKeyIssuerTheoryData
{
TestId = "Fails_With_Multiple_tids",
SecurityKey = KeyingMaterial.JsonWebKeyP256,
SecurityToken = new JsonWebToken(
Default.Jwt(Default.SecurityTokenDescriptor(
Default.SymmetricSigningCredentials,
[tidClaim, issClaim, new Claim("TID", Guid.NewGuid().ToString())]))),
ExpectedException = ExpectedException.SecurityTokenInvalidIssuerException("IDX40011"),
OpenIdConnectConfiguration = mockConfiguration
});

theoryData.Add(new AadSigningKeyIssuerTheoryData
{
TestId = "Fails_With_Multiple_tids_alternate_order",
SecurityKey = KeyingMaterial.JsonWebKeyP256,
SecurityToken = new JsonWebToken(
Default.Jwt(Default.SecurityTokenDescriptor(
Default.SymmetricSigningCredentials,
[issClaim, new Claim("TID", Guid.NewGuid().ToString()), tidClaim]))),
ExpectedException = ExpectedException.SecurityTokenInvalidIssuerException("IDX40011"),
OpenIdConnectConfiguration = mockConfiguration
});

theoryData.Add(new AadSigningKeyIssuerTheoryData
{
TestId = "Fails_With_no standard_tid",
SecurityKey = KeyingMaterial.JsonWebKeyP256,
SecurityToken = new JsonWebToken(
Default.Jwt(Default.SecurityTokenDescriptor(
Default.SymmetricSigningCredentials,
[issClaim, new Claim("TID", Guid.NewGuid().ToString())]))),
ExpectedException = ExpectedException.SecurityTokenInvalidIssuerException("IDX40009"),
OpenIdConnectConfiguration = mockConfiguration
});

return theoryData;
}
}
Expand All @@ -346,6 +454,10 @@ public class AadSigningKeyIssuerTheoryData : TheoryDataBase
public bool SetDelegateUsingConfig { get; set; } = false;

public bool SetDelegateWithoutConfig { get; set; } = false;

public Action SetupAction { get; set; }

public Action TearDownAction { get; set; }
}
}
}
Expand Down

0 comments on commit 0d5b3bf

Please sign in to comment.