Regression tests: Audience #2838

iNinja · 2024-09-23T13:51:28Z

Regression tests: Audience

Extracted regression tests related to audience to a separate file.
Added extra scenarios for audience related validations.
Fixed audience validation unit tests and renamed variables to improve clarity.

Note: Upcoming PRs will continue to extract the different aspects of regression tests into separate files.

Part of #2711

…rity.

…ded remaining regression tests for audience scenarios.

...ft.IdentityModel.JsonWebTokens.Tests/JsonWebTokenHandler.ValidateTokenAsyncTests.Audience.cs

sruke · 2024-09-24T13:12:24Z

...ft.IdentityModel.JsonWebTokens.Tests/JsonWebTokenHandler.ValidateTokenAsyncTests.Audience.cs

+                    {
+                        TokenValidationParameters = CreateTokenValidationParameters(null),
+                        ValidationParameters = CreateValidationParameters(null),
+                        Audience = string.Empty,


nit: Should this be a valid Audience while the Audiences property on TVP or VP is null?

The current validation path considers the case where no valid audiences are provided to be invalid and throws.
The new validation path took that as a base and kept the same behaviour.

If you believe that having no valid audiences should be considered the same as skipping the validation, we can look into it and find out what the correct behaviour would be.

...ft.IdentityModel.JsonWebTokens.Tests/JsonWebTokenHandler.ValidateTokenAsyncTests.Audience.cs

…lidations but audience

src/Microsoft.IdentityModel.Tokens/Validation/Results/Details/AudienceValidationError.cs

src/Microsoft.IdentityModel.Tokens/Validation/Validators.SkipValidations.cs

…ceptions out of the constructor

jennyf19

can we open an issue to track the changes to the IDX??? values so we can revisit before releasing?

brentschmaltz · 2024-09-30T17:50:45Z

src/Microsoft.IdentityModel.Tokens/Validation/Results/Details/AudienceValidationError.cs

+            _invalidAudiences = invalidAudiences;
+        }
+
+        internal override void AddAdditionalInformation(ISecurityTokenException exception)


I suspect that since we are creating derived ValidationError types, we do not need the method AddAdditionalInformation or the interface ISecurityTokenException.

For example, when we create the AudienceValidationError, we call the ctor that takes the list of available audiences.

Let’s keep the conversation going on that to find the best solution that can support any exception our users may decide to return.
I’d rather not expand the scope of this PR to that as it’s already a week in review.

iNinja added 2 commits September 23, 2024 14:47

Fixed audience validator unit tests. Renamed variables to improve cla…

1296457

…rity.

Extracted regression tests related to audience to a separate file. Ad…

76e6b9b

…ded remaining regression tests for audience scenarios.

iNinja requested a review from a team as a code owner September 23, 2024 13:51

jennyf19 reviewed Sep 24, 2024

View reviewed changes

...ft.IdentityModel.JsonWebTokens.Tests/JsonWebTokenHandler.ValidateTokenAsyncTests.Audience.cs Outdated Show resolved Hide resolved

jennyf19 reviewed Sep 24, 2024

View reviewed changes