Skip to content

Commit 9e7e701

Browse files
iarekkCopilot
andcommitted
Reapply "Throw on Authority vs Instance/TenantId conflict (#3873)"
Re-add the changes introduced in #3873 and reverted in #3888. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 2c8a9c2 commit 9e7e701

6 files changed

Lines changed: 668 additions & 79 deletions

File tree

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

Lines changed: 58 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,42 @@ public ConfidentialClientApplicationOptions ConfidentialClientApplicationOptions
7777
*/
7878
internal bool PreserveAuthority { get; set; }
7979

80+
// Latch: once true, never reverts to false. Distinguishes user-configured Authority
81+
// from the synthetic value computed by MicrosoftEntraApplicationOptions.Authority getter.
82+
private bool _authorityExplicitlyConfigured;
83+
internal bool AuthorityExplicitlyConfigured
84+
{
85+
get => _authorityExplicitlyConfigured;
86+
set { if (value) _authorityExplicitlyConfigured = true; }
87+
}
88+
89+
// Latch: once true, never reverts to false. Distinguishes user-configured
90+
// Instance/TenantId from values derived by ParseAuthorityIfNecessary.
91+
private bool _instanceOrTenantIdExplicitlyConfigured;
92+
internal bool InstanceOrTenantIdExplicitlyConfigured
93+
{
94+
get => _instanceOrTenantIdExplicitlyConfigured;
95+
set { if (value) _instanceOrTenantIdExplicitlyConfigured = true; }
96+
}
97+
98+
// Set after ParseAuthorityIfNecessary runs; makes subsequent calls no-op.
99+
private bool _authorityParsed;
100+
101+
// Throws if the user explicitly configured BOTH Authority AND Instance/TenantId.
102+
internal void ThrowIfAuthorityConflict()
103+
{
104+
if (AuthorityExplicitlyConfigured &&
105+
InstanceOrTenantIdExplicitlyConfigured &&
106+
!string.IsNullOrEmpty(Authority) &&
107+
(!string.IsNullOrEmpty(Instance) || !string.IsNullOrEmpty(TenantId)))
108+
{
109+
throw new InvalidOperationException(
110+
$"[MsIdWeb] Both 'Authority' ('{Authority}') and 'Instance'/'TenantId' " +
111+
$"('{Instance ?? string.Empty}', '{TenantId ?? string.Empty}') are configured. " +
112+
"These settings conflict. Remove either 'Authority' or 'Instance'/'TenantId' from the configuration.");
113+
}
114+
}
115+
80116
/// <summary>
81117
/// Id Web will modify the instance so that it can be used by MSAL.
82118
/// This modifies this property so that the original value is not changed.
@@ -281,6 +317,7 @@ internal static void UpdateMergedOptionsFromMicrosoftIdentityOptions(MicrosoftId
281317
if (string.IsNullOrEmpty(mergedOptions.Instance) && !string.IsNullOrEmpty(microsoftIdentityOptions.Instance))
282318
{
283319
mergedOptions.Instance = microsoftIdentityOptions.Instance;
320+
mergedOptions.InstanceOrTenantIdExplicitlyConfigured = true;
284321
}
285322

286323
if (microsoftIdentityOptions.ResetPasswordPath != Constants.ResetPasswordPath)
@@ -298,6 +335,7 @@ internal static void UpdateMergedOptionsFromMicrosoftIdentityOptions(MicrosoftId
298335
if (string.IsNullOrEmpty(mergedOptions.Authority) && !string.IsNullOrEmpty(microsoftIdentityOptions.Authority))
299336
{
300337
mergedOptions.Authority = microsoftIdentityOptions.Authority;
338+
mergedOptions.AuthorityExplicitlyConfigured = true;
301339
}
302340

303341
mergedOptions.ClientCredentials ??= microsoftIdentityOptions.ClientCredentials;
@@ -339,6 +377,7 @@ internal static void UpdateMergedOptionsFromMicrosoftIdentityOptions(MicrosoftId
339377
if (string.IsNullOrEmpty(mergedOptions.TenantId) && !string.IsNullOrEmpty(microsoftIdentityOptions.TenantId))
340378
{
341379
mergedOptions.TenantId = microsoftIdentityOptions.TenantId;
380+
mergedOptions.InstanceOrTenantIdExplicitlyConfigured = true;
342381
}
343382

344383
mergedOptions.TokenDecryptionCertificates ??= microsoftIdentityOptions.TokenDecryptionCertificates;
@@ -393,6 +432,7 @@ internal static void UpdateMergedOptionsFromConfidentialClientApplicationOptions
393432
if (string.IsNullOrEmpty(mergedOptions.Instance) && !string.IsNullOrEmpty(confidentialClientApplicationOptions.Instance))
394433
{
395434
mergedOptions.Instance = confidentialClientApplicationOptions.Instance;
435+
mergedOptions.InstanceOrTenantIdExplicitlyConfigured = true;
396436
}
397437

398438
mergedOptions.IsDefaultPlatformLoggingEnabled |= confidentialClientApplicationOptions.IsDefaultPlatformLoggingEnabled;
@@ -407,6 +447,7 @@ internal static void UpdateMergedOptionsFromConfidentialClientApplicationOptions
407447
if (string.IsNullOrEmpty(mergedOptions.TenantId) && !string.IsNullOrEmpty(confidentialClientApplicationOptions.TenantId))
408448
{
409449
mergedOptions.TenantId = confidentialClientApplicationOptions.TenantId;
450+
mergedOptions.InstanceOrTenantIdExplicitlyConfigured = true;
410451
}
411452

412453
mergedOptions._confidentialClientApplicationOptions = null;
@@ -473,22 +514,20 @@ internal static void UpdateConfidentialClientApplicationOptionsFromMergedOptions
473514
*/
474515
internal static void ParseAuthorityIfNecessary(MergedOptions mergedOptions, IdWebLogger.ILogger? logger = null)
475516
{
476-
// Check if Authority is configured but being ignored due to Instance/TenantId taking precedence
477-
if (!string.IsNullOrEmpty(mergedOptions.Authority) &&
478-
(!string.IsNullOrEmpty(mergedOptions.Instance) || !string.IsNullOrEmpty(mergedOptions.TenantId)))
479-
{
480-
// Log warning that Authority is being ignored
481-
if (logger != null)
482-
{
483-
MergedOptionsLogging.AuthorityIgnored(
484-
logger,
485-
mergedOptions.Authority!,
486-
mergedOptions.Instance ?? string.Empty,
487-
mergedOptions.TenantId ?? string.Empty);
488-
}
489-
// Authority is ignored; Instance and TenantId take precedence
490-
return;
491-
}
517+
if (mergedOptions._authorityParsed)
518+
{
519+
return;
520+
}
521+
522+
if (!string.IsNullOrEmpty(mergedOptions.Authority) &&
523+
(!string.IsNullOrEmpty(mergedOptions.Instance) || !string.IsNullOrEmpty(mergedOptions.TenantId)))
524+
{
525+
mergedOptions.ThrowIfAuthorityConflict();
526+
527+
// Authority was synthesized (e.g. by MicrosoftEntraApplicationOptions computed getter) -- not a real conflict.
528+
mergedOptions._authorityParsed = true;
529+
return;
530+
}
492531

493532
if (string.IsNullOrEmpty(mergedOptions.TenantId) && string.IsNullOrEmpty(mergedOptions.Instance) && !string.IsNullOrEmpty(mergedOptions.Authority))
494533
{
@@ -543,6 +582,8 @@ internal static void ParseAuthorityIfNecessary(MergedOptions mergedOptions, IdWe
543582
mergedOptions.Instance = mergedOptions.PreserveAuthority ? mergedOptions.Authority! : authoritySpan.Slice(0, indexTenant).ToString();
544583
mergedOptions.TenantId = mergedOptions.PreserveAuthority ? null : authoritySpan.Slice(indexTenant + 1, indexEndOfTenant - indexTenant - 1).ToString();
545584
}
585+
586+
mergedOptions._authorityParsed = true;
546587
}
547588
}
548589

@@ -552,6 +593,7 @@ internal static void UpdateMergedOptionsFromJwtBearerOptions(JwtBearerOptions jw
552593
if (string.IsNullOrEmpty(mergedOptions.Authority) && !string.IsNullOrEmpty(jwtBearerOptions.Authority))
553594
{
554595
mergedOptions.Authority = jwtBearerOptions.Authority;
596+
mergedOptions.AuthorityExplicitlyConfigured = true;
555597
}
556598
}
557599
#endif

src/Microsoft.Identity.Web/WebAppExtensions/MicrosoftIdentityWebAppAuthenticationBuilderExtensions.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,9 @@ s.ServiceKey is null &&
322322

323323
MergedOptionsValidation.Validate(mergedOptions);
324324

325+
// Throw early if Authority conflicts with Instance/TenantId (same check as MSAL path).
326+
mergedOptions.ThrowIfAuthorityConflict();
327+
325328
if (mergedOptions.Authority != null)
326329
{
327330
mergedOptions.Authority = AuthorityHelpers.GetAuthorityWithoutQueryIfNeeded(mergedOptions);

0 commit comments

Comments
 (0)