Skip to content

Commit c33e90f

Browse files
Check for null in PrepareAuthorityInstanceForMsal (#3338)
1 parent 36fb5f5 commit c33e90f

2 files changed

Lines changed: 63 additions & 5 deletions

File tree

src/Microsoft.Identity.Web.TokenAcquisition/MergedOptions.cs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -473,9 +473,14 @@ internal static void UpdateMergedOptionsFromJwtBearerOptions(JwtBearerOptions jw
473473

474474
public void PrepareAuthorityInstanceForMsal()
475475
{
476+
if (string.IsNullOrEmpty(Instance))
477+
{
478+
return;
479+
}
480+
476481
if (IsB2C && Instance.EndsWith("/tfp/", StringComparison.OrdinalIgnoreCase))
477482
{
478-
#if !NETSTANDARD2_0 && !NET462 && !NET472
483+
#if NETCOREAPP
479484
PreparedInstance = Instance.Replace("/tfp/", string.Empty, StringComparison.OrdinalIgnoreCase).TrimEnd('/') + "/";
480485
#else
481486
PreparedInstance = Instance.Replace("/tfp/", string.Empty).TrimEnd('/') + "/";

tests/Microsoft.Identity.Web.Test/MergedOptionsTests.cs

Lines changed: 57 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ namespace Microsoft.Identity.Web.Test
1010
{
1111
public class MergedOptionsTests
1212
{
13-
// appliation options
13+
// application options
1414
private readonly string _appOptionsAuthority = "microsoftIdentityApplicationOptionsAuthority";
1515
private readonly string _appOptionsAzureRegion = "microsoftIdentityApplicationOptionsAzureRegion";
1616
private readonly string[] _appOptionsClientCapabilities = new string[] { "microsoftIdentityApplicationOptionsClientCapabilities" };
@@ -24,7 +24,7 @@ public class MergedOptionsTests
2424
private readonly string _appOptionsPasswordRestId = "microsoftIdentityApplicationOptionsResetPasswordPolicyId";
2525
private readonly string _appOptionsSuSiPolicyId = "microsoftIdentityApplicationOptionsSignUpSignInPolicyId";
2626
private readonly string _appOptionsTenantId = "microsoftIdentityApplicationOptionsTenantId";
27-
private readonly IEnumerable<CredentialDescription> _appOptionsTokenDecyrptCreds = new CredentialDescription[] { new CredentialDescription() };
27+
private readonly IEnumerable<CredentialDescription> _appOptionsTokenDecryptCreds = new CredentialDescription[] { new CredentialDescription() };
2828

2929
// MS Identity options
3030
private readonly string _msIdentityOptionsAccessDeniedPath = "/microsoftIdentityOptionsAccessDeniedPath";
@@ -85,7 +85,7 @@ public void UpdateMergedOptionsFromMicrosoftIdentityApplicationOptions_Then_Defa
8585
microsoftIdentityApplicationOptions.SendX5C = true;
8686
microsoftIdentityApplicationOptions.SignUpSignInPolicyId = _appOptionsSuSiPolicyId;
8787
microsoftIdentityApplicationOptions.TenantId = _appOptionsTenantId;
88-
microsoftIdentityApplicationOptions.TokenDecryptionCredentials = _appOptionsTokenDecyrptCreds;
88+
microsoftIdentityApplicationOptions.TokenDecryptionCredentials = _appOptionsTokenDecryptCreds;
8989
microsoftIdentityApplicationOptions.WithSpaAuthCode = true;
9090

9191
// Act
@@ -118,7 +118,7 @@ public void UpdateMergedOptionsFromMicrosoftIdentityApplicationOptions_Then_Defa
118118
Assert.True(mergedOptions.SendX5C);
119119
Assert.Equal(_appOptionsSuSiPolicyId, mergedOptions.SignUpSignInPolicyId);
120120
Assert.Equal(_appOptionsTenantId, mergedOptions.TenantId);
121-
Assert.Equal(_appOptionsTokenDecyrptCreds, mergedOptions.TokenDecryptionCredentials!);
121+
Assert.Equal(_appOptionsTokenDecryptCreds, mergedOptions.TokenDecryptionCredentials!);
122122
Assert.True(mergedOptions.WithSpaAuthCode);
123123
}
124124

@@ -239,5 +239,58 @@ public void UpdateMergedOptionsFromMicrosoftIdentityOptions_ThenMicrosoftIdentit
239239
Assert.True(mergedOptions.UseTokenLifetime);
240240
Assert.True(mergedOptions.WithSpaAuthCode);
241241
}
242+
243+
[Theory]
244+
[InlineData("https://login.microsoftonline.com", "https://login.microsoftonline.com/")]
245+
[InlineData("https://login.microsoftonline.com/", "https://login.microsoftonline.com/")] // Already has trailing slash
246+
public void PrepareAuthorityInstanceForMsal_PreparesInstance_WhenInstanceIsSet(
247+
string instance,
248+
string expectedPreparedInstance)
249+
{
250+
// Arrange
251+
var options = new MergedOptions { Instance = instance, TenantId = "common" }; // TenantId is set, so Authority shouldn't be parsed
252+
253+
// Act
254+
options.PrepareAuthorityInstanceForMsal();
255+
256+
// Assert
257+
Assert.Equal(expectedPreparedInstance, options.PreparedInstance);
258+
Assert.Equal(instance, options.Instance); // Original Instance remains unchanged
259+
}
260+
261+
[Fact]
262+
public void PrepareAuthorityInstanceForMsal_DoesNothing_WhenAuthorityAndInstanceAreNull()
263+
{
264+
// Arrange
265+
var options = new MergedOptions();
266+
267+
// Act
268+
options.PrepareAuthorityInstanceForMsal();
269+
270+
// Assert
271+
Assert.Null(options.Instance);
272+
Assert.Null(options.TenantId);
273+
Assert.Null(options.PreparedInstance);
274+
}
275+
276+
[Fact]
277+
public void PrepareAuthorityInstanceForMsal_DoesNotParseAuthority_WhenInstanceAndTenantIdAreSet()
278+
{
279+
// Arrange
280+
var options = new MergedOptions
281+
{
282+
Authority = "https://login.microsoftonline.com/common", // Should be ignored
283+
Instance = "https://login.microsoftonline.us/",
284+
TenantId = "organizations"
285+
};
286+
287+
// Act
288+
options.PrepareAuthorityInstanceForMsal();
289+
290+
// Assert
291+
Assert.Equal("https://login.microsoftonline.us/", options.Instance); // Instance remains unchanged
292+
Assert.Equal("organizations", options.TenantId); // TenantId remains unchanged
293+
Assert.Equal("https://login.microsoftonline.us/", options.PreparedInstance); // PreparedInstance based on original Instance
294+
}
242295
}
243296
}

0 commit comments

Comments
 (0)