Skip to content

Fixing IsCommonOrOrganizationsTenant check to not return true for consumers #5195

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

Merged
merged 5 commits into from
Mar 20, 2025
+52 −16
Merged

Fixing IsCommonOrOrganizationsTenant check to not return true for consumers #5195

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
117a914
Fixing IsCommonOrOrganizationsTenant check to not return true for con…
Mar 18, 2025
f0b557d
Updating tests to better validate IsCommonOrOrganizationsTenant
Mar 19, 2025
ce9e58b
Removing extra test
Mar 19, 2025
dc31ddf
Merge branch 'main' into trwalke/IsOrgOrCommmonAuthorityFix
trwalke Mar 19, 2025
aa343fc
Test update
Mar 19, 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: 2 additions & 1 deletion src/client/Microsoft.Identity.Client/Instance/AadAuthority.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ internal bool IsCommonOrOrganizationsTenant()

internal static bool IsCommonOrOrganizationsTenant(string tenantId)
{
return !string.IsNullOrEmpty(tenantId) &&
return !string.IsNullOrEmpty(tenantId) &&
!IsConsumers(tenantId) &&
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice investigation in #5184 to find out this regression was introduced by #3517. Strangely, that change in #3517 received an explicit upvote, so, it seemed like an intentional and even desirable removal. I don't remember any context of this change off the top of my head.

s_tenantlessTenantNames.Contains(tenantId);
}

Expand Down
28 changes: 28 additions & 0 deletions tests/Microsoft.Identity.Test.Unit/CoreTests/InstanceTests/AadAuthorityTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,34 @@ public void IsOrganizationsTenantWithMsaPassthroughEnabled()
Assert.IsTrue(aadAuthorityInstance.IsOrganizationsTenantWithMsaPassthroughEnabled(true, "9188040d-6c67-4c5b-b112-36a304b66dad"));
}

[TestMethod]
public void IsCommonOrOrganizationsTenantTest()
{
//Test for common tenant
AadAuthority aadAuthorityInstance = new AadAuthority(Authority.CreateAuthority("https://login.microsoftonline.com/common").AuthorityInfo);

Assert.IsNotNull(aadAuthorityInstance);
Assert.AreEqual(aadAuthorityInstance.AuthorityInfo.AuthorityType, AuthorityType.Aad);

Assert.IsTrue(aadAuthorityInstance.IsCommonOrOrganizationsTenant());

//Test for common Organizations tenant
aadAuthorityInstance = new AadAuthority(Authority.CreateAuthority("https://login.microsoftonline.com/organizations").AuthorityInfo);

Assert.IsNotNull(aadAuthorityInstance);
Assert.AreEqual(aadAuthorityInstance.AuthorityInfo.AuthorityType, AuthorityType.Aad);

Assert.IsTrue(aadAuthorityInstance.IsCommonOrOrganizationsTenant());

//Test for common Organizations tenant
aadAuthorityInstance = new AadAuthority(Authority.CreateAuthority("https://login.microsoftonline.com/consumers").AuthorityInfo);

Assert.IsNotNull(aadAuthorityInstance);
Assert.AreEqual(aadAuthorityInstance.AuthorityInfo.AuthorityType, AuthorityType.Aad);

Assert.IsFalse(aadAuthorityInstance.IsCommonOrOrganizationsTenant());
}

[TestMethod]
public async Task CreateAuthorityForRequestAsync_MSAPassthroughAsync()
{
Expand Down
Loading