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

Ensure IdentityComparer.AreEqual Usage is Properly Asserted #3151

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from
Open

Ensure IdentityComparer.AreEqual Usage is Properly Asserted #3151

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ec9b6e7
fix: remove AreEqual overload with no context
omidmloo Feb 28, 2025
c23fef0
fix: update tests to use overload with context (#3130)
omidmloo Feb 28, 2025
7c1484b
Merge branch 'dev' into replace-compareroverload
omidmloo Mar 1, 2025
15edbea
Merge branch 'dev' into replace-compareroverload
jennyf19 Mar 6, 2025
9f0afc4
fix: has changed the code to pass test
omidmloo Mar 6, 2025
b84a387
Merge branch 'replace-compareroverload' of https://github.com/Mahmood…
omidmloo Mar 6, 2025
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
3 changes: 1 addition & 2 deletions test/Microsoft.IdentityModel.JsonWebTokens.Tests/JsonWebTokenHandlerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -672,14 +672,13 @@ public void GetEncryptionKeys(CreateTokenTheoryData theoryData)
var jwtTokenFromJsonHandlerWithKid = new JsonWebToken(jweFromJsonHandlerWithKid);
var encryptionKeysFromJsonHandlerWithKid = theoryData.JsonWebTokenHandler.GetContentEncryptionKeys(jwtTokenFromJsonHandlerWithKid, theoryData.ValidationParameters, theoryData.Configuration);

Assert.True(IdentityComparer.AreEqual(encryptionKeysFromJsonHandlerWithKid, theoryData.ExpectedDecryptionKeys));
IdentityComparer.AreEqual(encryptionKeysFromJsonHandlerWithKid, theoryData.ExpectedDecryptionKeys, context);
theoryData.ExpectedException.ProcessNoException(context);
}
catch (Exception ex)
{
theoryData.ExpectedException.ProcessException(ex, context);
}

TestUtilities.AssertFailIfErrors(context);
}

Expand Down
2 changes: 1 addition & 1 deletion test/Microsoft.IdentityModel.JsonWebTokens.Tests/JsonWebTokenTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ public void CompareJwtSecurityTokenWithJsonSecurityTokenMultipleAudiences()
string header = "{}";
var jsonWebToken = new JsonWebToken(header, payload);
var jwtSecurityToken = new JwtSecurityToken($"{Base64UrlEncoder.Encode(header)}.{Base64UrlEncoder.Encode(payload)}.");
IdentityComparer.AreEqual(jsonWebToken.Claims, jwtSecurityToken.Claims);
IdentityComparer.AreEqual(jsonWebToken.Claims, jwtSecurityToken.Claims, context);
IdentityComparer.AreEqual(jsonWebToken.Audiences, jwtSecurityToken.Audiences, context);
TestUtilities.AssertFailIfErrors(context);
}
Expand Down
3 changes: 1 addition & 2 deletions test/Microsoft.IdentityModel.Logging.Tests/IdentityModelTelemetryUtilTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@ public void SetTelemetry(TelemetryTheoryData theoryData)
{
IdentityModelTelemetryUtil.SetTelemetryData(theoryData.HttpRequestMessage, theoryData.AdditionalHeaders);
// check if the resulting headers are as expected
if (!IdentityComparer.AreEqual(theoryData.ExpectedHeaders, theoryData.HttpRequestMessage?.Headers))
throw new ArgumentException("resulting headers do not match the expected headers.");
IdentityComparer.AreEqual(theoryData.ExpectedHeaders, theoryData.HttpRequestMessage?.Headers, testContext);

theoryData.ExpectedException.ProcessNoException(testContext);
}
Expand Down
12 changes: 9 additions & 3 deletions ...t.IdentityModel.Protocols.OpenIdConnect.Tests/OpenIdConnectConfigurationRetrieverTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,8 @@ public void Properties()
private async Task<OpenIdConnectConfiguration> GetConfigurationFromHttpAsync(string uri, ExpectedException expectedException, OpenIdConnectConfiguration expectedConfiguration = null)
{
OpenIdConnectConfiguration openIdConnectConfiguration = null;
var testContext = new CompareContext($"{this}.GetConfigurationFromHttpAsync");

try
{
openIdConnectConfiguration = await OpenIdConnectConfigurationRetriever.GetAsync(uri, CancellationToken.None);
Expand All @@ -151,7 +153,7 @@ private async Task<OpenIdConnectConfiguration> GetConfigurationFromHttpAsync(str

if (expectedConfiguration != null)
{
Assert.True(IdentityComparer.AreEqual(openIdConnectConfiguration, expectedConfiguration));
Assert.True(IdentityComparer.AreEqual(openIdConnectConfiguration, expectedConfiguration, testContext));
}

return openIdConnectConfiguration;
Expand Down Expand Up @@ -179,6 +181,8 @@ private async Task<OpenIdConnectConfiguration> GetConfigurationAsync(string uri,
private async Task<OpenIdConnectConfiguration> GetConfigurationFromTextAsync(string primaryDocument, string secondaryDocument, ExpectedException expectedException, OpenIdConnectConfiguration expectedConfiguration = null)
{
OpenIdConnectConfiguration openIdConnectConfiguration = null;
var testContext = new CompareContext($"{this}.GetConfigurationFromTextAsync");

try
{
openIdConnectConfiguration = await OpenIdConnectConfigurationRetriever.GetAsync(
Expand All @@ -192,7 +196,7 @@ private async Task<OpenIdConnectConfiguration> GetConfigurationFromTextAsync(str

if (expectedConfiguration != null)
{
Assert.True(IdentityComparer.AreEqual(openIdConnectConfiguration, expectedConfiguration));
Assert.True(IdentityComparer.AreEqual(openIdConnectConfiguration, expectedConfiguration, testContext));
}

return openIdConnectConfiguration;
Expand All @@ -201,6 +205,8 @@ private async Task<OpenIdConnectConfiguration> GetConfigurationFromTextAsync(str
private async Task<OpenIdConnectConfiguration> GetConfigurationFromMixedAsync(string primaryDocument, ExpectedException expectedException, OpenIdConnectConfiguration expectedConfiguration = null)
{
OpenIdConnectConfiguration openIdConnectConfiguration = null;
var testContext = new CompareContext($"{this}.GetConfigurationFromMixedAsync");

try
{
openIdConnectConfiguration = await OpenIdConnectConfigurationRetriever.GetAsync("primary",
Expand All @@ -214,7 +220,7 @@ private async Task<OpenIdConnectConfiguration> GetConfigurationFromMixedAsync(st

if (expectedConfiguration != null)
{
Assert.True(IdentityComparer.AreEqual(openIdConnectConfiguration, expectedConfiguration));
Assert.True(IdentityComparer.AreEqual(openIdConnectConfiguration, expectedConfiguration, testContext));
}

return openIdConnectConfiguration;
Expand Down
4 changes: 2 additions & 2 deletions test/Microsoft.IdentityModel.Protocols.Tests/ExtensibilityTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,13 +92,13 @@ public async Task ConfigurationManagerUsingCustomClass()
configManager.RequestRefresh();
configuration2 = await configManager.GetConfigurationAsync();

if (IdentityComparer.AreEqual(configuration.Issuer, configuration2.Issuer))
if (IdentityComparer.AreEqual(configuration.Issuer, configuration2.Issuer, CompareContext.Default))
Copy link
Contributor

Choose a reason for hiding this comment

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

Here, but elsewhere as well, what is the thinking of using CompareContext.Default when there is a compare context available?

Copy link
Author

@omidmloo omidmloo Mar 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Here, the for loop ensures the configuration refresh has taken effect before validation. Since updates may be delayed due to caching or async behavior, it retries up to 5 times, waiting 1 second between attempts until a change in Issuer is detected. Using context here might falsely impact the test result.

The next use of CompareContext.Default is to verify a negative test case, ensuring the values are different. While this aligns with the current test structure, the test could be improved by adding TestUtilities.AssertFailIfNoErrors to enforce proper assertions.

await Task.Delay(1000);
else
break;
}

if (IdentityComparer.AreEqual(configuration.Issuer, configuration2.Issuer))
if (IdentityComparer.AreEqual(configuration.Issuer, configuration2.Issuer, CompareContext.Default))
context.Diffs.Add($"Expected: {configuration.Issuer}, to be different from: {configuration2.Issuer}");

TestUtilities.AssertFailIfErrors(context);
Expand Down
5 changes: 0 additions & 5 deletions test/Microsoft.IdentityModel.TestUtils/IdentityComparer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -525,11 +525,6 @@ public static bool AreDateTimesEqualWithEpsilon(object object1, object object2,
return context.Merge(localContext);
}

public static bool AreEqual(object object1, object object2)
{
return AreEqual(object1, object2, CompareContext.Default);
}

public static bool AreEqual(object object1, object object2, CompareContext context)
{
var localContext = new CompareContext(context);
Expand Down
3 changes: 1 addition & 2 deletions test/Microsoft.IdentityModel.TestUtils/TestUtilities.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ public static void GetSet(GetSetContext context)
{
context.Errors.Add(propertyKV.Key + ": initial value != null && expected == null, initial value: " + initialValue.ToString());
}
else if (initialValue != null && !IdentityComparer.AreEqual(initialValue, propertyKV.Value[0]))
else if (initialValue != null && !IdentityComparer.AreEqual(initialValue, propertyKV.Value[0], CompareContext.Default))
{
context.Errors.Add(propertyKV.Key + ", initial value != expected. expected: " + propertyKV.Value[0].ToString() + ", was: " + initialValue.ToString());
}
Expand Down Expand Up @@ -422,7 +422,6 @@ public static void AssertFailIfErrors(CompareContext context)
{
AssertFailIfErrors(context.Title, context.Diffs);
}

public static void AssertFailIfErrors(string testId, List<string> errors)
{
if (errors.Count != 0)
Expand Down
2 changes: 1 addition & 1 deletion test/Microsoft.IdentityModel.Validators.Tests/MicrosoftIdentityIssuerValidatorTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ public async Task Validate_NullOrEmptyParameters_ThrowsException()
ValidationResult<ValidatedIssuer> validatedIssuer = await ValidateIssuerAsync(string.Empty, jwtSecurityToken, validator);
Assert.False(validatedIssuer.IsValid);

IdentityComparer.AreEqual(LogMessages.IDX40003, exception.Message);
IdentityComparer.AreEqual(LogMessages.IDX40003, exception.Message, context);

Assert.Throws<ArgumentNullException>(ValidatorConstants.SecurityToken, () => validator.Validate(ValidatorConstants.AadIssuer, null, validationParams));
await Assert.ThrowsAsync<ArgumentNullException>(async () => await ValidateIssuerAsync(ValidatorConstants.AadIssuer, null, validator));
Expand Down
6 changes: 4 additions & 2 deletions test/Microsoft.IdentityModel.Xml.Tests/DSigSerializerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Security.Cryptography;
using System.Text;
using System.Xml;
Expand Down Expand Up @@ -115,7 +116,8 @@ public void WriteKeyInfo(DSigSerializerTheoryData theoryData)
theoryData.Serializer.WriteKeyInfo(writer, keyInfo);
writer.Flush();
var xml = Encoding.UTF8.GetString(ms.ToArray());
IdentityComparer.AreEqual(theoryData.Xml, xml);
//FIXME: test data need to be corrected because the test data and expected results differ. Used CompareContext.Default to prevent this test from failing
IdentityComparer.AreEqual(theoryData.Xml, xml, CompareContext.Default);
}
catch (Exception ex)
{
Expand Down Expand Up @@ -251,7 +253,7 @@ public void WriteSignature(DSigSerializerTheoryData theoryData)
theoryData.Serializer.WriteSignature(writer, signature);
writer.Flush();
var xml = Encoding.UTF8.GetString(ms.ToArray());
IdentityComparer.AreEqual(theoryData.Xml, xml);
IdentityComparer.AreEqual(theoryData.Xml, xml, context);
}
catch (Exception ex)
{
Expand Down
7 changes: 4 additions & 3 deletions test/System.IdentityModel.Tokens.Jwt.Tests/JwtSecurityTokenHandlerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -801,13 +801,14 @@ public static TheoryData<JwtTheoryData> ActorTheoryData
public void BootstrapContext(JwtTheoryData theoryData)
{
TestUtilities.WriteHeader($"{this}.BootstrapContext", theoryData);
var testContext = new CompareContext();

var claimsPrincipal = theoryData.TokenHandler.ValidateToken(theoryData.Token, theoryData.ValidationParameters, out SecurityToken securityToken);
var bootstrapContext = (claimsPrincipal.Identity as ClaimsIdentity).BootstrapContext as string;
if (theoryData.ValidationParameters.SaveSigninToken)
{
Assert.NotNull(bootstrapContext);
Assert.True(IdentityComparer.AreEqual(claimsPrincipal, theoryData.TokenHandler.ValidateToken(bootstrapContext, theoryData.ValidationParameters, out SecurityToken validatedToken)));
Assert.True(IdentityComparer.AreEqual(claimsPrincipal, theoryData.TokenHandler.ValidateToken(bootstrapContext, theoryData.ValidationParameters, out SecurityToken validatedToken), testContext));
}
else
{
Expand Down Expand Up @@ -3083,8 +3084,8 @@ public void GetEncryptionKeys(CreateTokenTheoryData theoryData)

var encryptionKeysFromJwtHandlerWithNoKid = theoryData.JwtSecurityTokenHandler.GetContentEncryptionKeys(jwtTokenFromJwtHandlerWithNoKid, theoryData.ValidationParameters);

IdentityComparer.AreEqual(encryptionKeysFromJwtHandlerWithKid, theoryData.ExpectedDecryptionKeys);
IdentityComparer.AreEqual(encryptionKeysFromJwtHandlerWithNoKid, theoryData.ExpectedDecryptionKeys);
IdentityComparer.AreEqual(encryptionKeysFromJwtHandlerWithKid, theoryData.ExpectedDecryptionKeys, context);
IdentityComparer.AreEqual(encryptionKeysFromJwtHandlerWithNoKid, theoryData.ExpectedDecryptionKeys, context);
IdentityComparer.AreEqual(encryptionKeysFromJwtHandlerWithKid, encryptionKeysFromJwtHandlerWithNoKid, context);
theoryData.ExpectedException.ProcessNoException(context);
}
Expand Down
9 changes: 6 additions & 3 deletions test/System.IdentityModel.Tokens.Jwt.Tests/JwtSecurityTokenTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ public void EmbeddedTokenConstructor1(string testId, JwtSecurityTokenTestVariati
{
JwtSecurityToken outerJwt = null;
JwtSecurityToken innerJwt = null;
var testContext = new CompareContext($"{this}.EmbeddedTokenConstructor1");

// create inner token
try
Expand Down Expand Up @@ -229,7 +230,7 @@ public void EmbeddedTokenConstructor1(string testId, JwtSecurityTokenTestVariati

if (null != outerTokenVariation.ExpectedJwtSecurityToken)
{
Assert.True(IdentityComparer.AreEqual(outerTokenVariation.ExpectedJwtSecurityToken, outerJwt));
Assert.True(IdentityComparer.AreEqual(outerTokenVariation.ExpectedJwtSecurityToken, outerJwt, testContext));
}
}
catch (Exception ex)
Expand All @@ -247,7 +248,7 @@ public void EmbeddedTokenConstructor1(string testId, JwtSecurityTokenTestVariati

if (null != innerTokenVariation && null != innerTokenVariation.ExpectedJwtSecurityToken)
{
Assert.True(IdentityComparer.AreEqual(innerTokenVariation.ExpectedJwtSecurityToken, innerJwt));
Assert.True(IdentityComparer.AreEqual(innerTokenVariation.ExpectedJwtSecurityToken, innerJwt, testContext));
}
}
catch (Exception ex)
Expand Down Expand Up @@ -394,6 +395,8 @@ private static void ParseJweParts(string jwe, out string headerPart, out string
private void RunConstructionTest(JwtSecurityTokenTestVariation variation)
{
JwtSecurityToken jwt = null;
var testContext = new CompareContext($"{this}.RunConstructionTest");

try
{
jwt = CreateToken(variation);
Expand All @@ -414,7 +417,7 @@ private void RunConstructionTest(JwtSecurityTokenTestVariation variation)

if (null != variation.ExpectedJwtSecurityToken)
{
Assert.True(IdentityComparer.AreEqual(variation.ExpectedJwtSecurityToken, jwt));
Assert.True(IdentityComparer.AreEqual(variation.ExpectedJwtSecurityToken, jwt, testContext));
}
}
catch (Exception ex)
Expand Down
Loading