Skip to content

Commit

Permalink
Remove dependency on AadIssuerValidator.GetTenantIdFromToken in Valid…
Browse files Browse the repository at this point in the history 
…ateIssuerSigningKey (#2680) (#2686)

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

---------

Co-authored-by: Keegan Caruso <keegancaruso@microsoft.com>
  • Loading branch information
@keegan-caruso
keegan-caruso and Keegan Caruso authored Jul 9, 2024
1 parent a93b7f6 commit eaeb748
Show file tree
Hide file tree
Showing 5 changed files with 182 additions and 4 deletions.
1 change: 1 addition & 0 deletions src/Microsoft.IdentityModel.JsonWebTokens/InternalsVisibleTo.cs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

[assembly: System.Runtime.CompilerServices.InternalsVisibleTo("Microsoft.IdentityModel.Validators, PublicKey=0024000004800000940000000602000000240000525341310004000001000100b5fc90e7027f67871e773a8fde8938c81dd402ba65b9201d60593e96c492651e889cc13f1415ebb53fac1131ae0bd333c5ee6021672d9718ea31a8aebd0da0072f25d87dba6fc90ffd598ed4da35e44c398c454307e8e33b8426143daec9f596836f97c8f74750e5975c64e2189f45def46b2a2b1247adc3652bf5c308055da9")]
[assembly: System.Runtime.CompilerServices.InternalsVisibleTo("Microsoft.IdentityModel.JsonWebTokens.Tests, PublicKey=0024000004800000940000000602000000240000525341310004000001000100b5fc90e7027f67871e773a8fde8938c81dd402ba65b9201d60593e96c492651e889cc13f1415ebb53fac1131ae0bd333c5ee6021672d9718ea31a8aebd0da0072f25d87dba6fc90ffd598ed4da35e44c398c454307e8e33b8426143daec9f596836f97c8f74750e5975c64e2189f45def46b2a2b1247adc3652bf5c308055da9")]
5 changes: 5 additions & 0 deletions src/Microsoft.IdentityModel.JsonWebTokens/JsonWebToken.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -625,6 +625,11 @@ public Claim GetClaim(string key)
return Payload.GetClaim(key, Issuer ?? ClaimsIdentity.DefaultIssuer);
}

/// <summary>
/// Gets the names of the payload claims on the JsonWebToken.
/// </summary>
internal IReadOnlyCollection<string> PayloadClaimNames => Payload._jsonClaims.Keys;

internal ClaimsIdentity ClaimsIdentity
{
get
Expand Down
64 changes: 61 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,11 @@
// Licensed under the MIT License.

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

internal const string DontFailOnMissingTidSwitch = "Switch.Microsoft.IdentityModel.DontFailOnMissingTidValidateIssuerSigning";

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

/// <summary>
/// Validates the issuer signing key.
/// </summary>
Expand All @@ -67,9 +78,14 @@ 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 (DontFailOnMissingTid())
return true;

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

var tokenIssuer = securityToken.Issuer;

Expand All @@ -91,7 +107,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 +117,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 @@ -24,5 +24,9 @@ internal static class LogMessages
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 IDX40008 = "IDX40008: When setting LastKnownGoodLifetime, the value must be greater than or equal to zero. value: '{0}'.";

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.";
}
}
112 changes: 111 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,94 @@ public static TheoryData<AadSigningKeyIssuerTheoryData> ValidateIssuerSigningKey
ExpectedException = ExpectedException.SecurityTokenInvalidIssuerException("IDX40004")
});

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)
});

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 +452,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 eaeb748

Please sign in to comment.