For Mtls, do not check for Aad and Dsts Authority type and 'organizations' or 'common' tenant #5174

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

Jump to bottom

Merged

ksaaf merged 8 commits into main from ksaaf/issue-5093

Mar 14, 2025

Contributor

ksaaf commented Mar 7, 2025 •

edited

Loading

This PR removes from AcquireTokenForClientParameterBuilder.Validate checks for 'common' for Aad and Dsts Authority Types.

Fixes #5093

Changes proposed in this request

Removes from AcquireTokenForClientParameterBuilder.Validate checks for 'common' for Aad and Dsts Authority Types.

Testing

Updates unit tests

Performance impact

N/A since it is removing code.

Documentation

All relevant documentation is updated.


          For Aad and Dsts Authority, raise error 'organizations' tenant like '…

6dd9225

…common'

ksaaf requested a review from a team as a code owner

March 7, 2025 00:36

ksaaf requested a review from gladjohn

March 7, 2025 00:37

Contributor Author

ksaaf commented Mar 7, 2025

@microsoft-github-policy-service agree company="Microsoft"

gladjohn reviewed

View reviewed changes

src/client/Microsoft.Identity.Client/AppConfig/AuthorityInfo.cs Outdated Show resolved Hide resolved

gladjohn reviewed

View reviewed changes

tests/Microsoft.Identity.Test.Unit/PublicApiTests/MtlsPopTests.cs Show resolved Hide resolved

gladjohn reviewed

View reviewed changes

tests/Microsoft.Identity.Test.Unit/PublicApiTests/MtlsPopTests.cs Show resolved Hide resolved

gladjohn reviewed

View reviewed changes

tests/Microsoft.Identity.Test.Unit/ApiConfigTests/AuthorityTests.cs Outdated Show resolved Hide resolved

gladjohn reviewed

View reviewed changes

Contributor

gladjohn left a comment

Looks good @ksaaf. From the tracking bug, I understand we need to make sure /consumers work. Do we have a test to make sure the happy path works for /consumers


          Address PR feedback: Add mixed-casing tests

d7e7aa3

Contributor Author

ksaaf commented Mar 7, 2025

Looks good @ksaaf. From the tracking bug, I understand we need to make sure /consumers work. Do we have a test to make sure the happy path works for /consumers

@gladjohn - Please see ConfidentialClientApplicationTests.GetAuthorizationRequestUrl_WithConsumerInCreate_ReturnsConsumers. Let me know if we are still missing a test case here.


          Address PR feedback: Rename tenantless-Tenant names

3b8642e

ksaaf requested a review from gladjohn

March 7, 2025 18:43

bgavrilMS reviewed

View reviewed changes

src/client/Microsoft.Identity.Client/ApiConfig/AcquireTokenForClientParameterBuilder.cs Outdated Show resolved Hide resolved

bgavrilMS reviewed

View reviewed changes

src/client/Microsoft.Identity.Client/ApiConfig/AcquireTokenForClientParameterBuilder.cs Show resolved Hide resolved

bgavrilMS reviewed

View reviewed changes

src/client/Microsoft.Identity.Client/ApiConfig/AcquireTokenForClientParameterBuilder.cs Show resolved Hide resolved

rayluo reviewed

View reviewed changes

src/client/Microsoft.Identity.Client/AppConfig/AuthorityInfo.cs Outdated Show resolved Hide resolved


          Move ThrowIfNotSupportedForMtls to Authority to simplify tenant check

81daa04

ksaaf commented

View reviewed changes

src/client/Microsoft.Identity.Client/Instance/Authority.cs Outdated Show resolved Hide resolved

ksaaf requested review from bgavrilMS and rayluo

March 11, 2025 18:34


          Merge branch 'main' into ksaaf/issue-5093

17fe6dd

AzureAD deleted a comment from kennyoye88

Kashif Mehmood added 2 commits

March 13, 2025 12:08


          Remove check for common/organization for Aad and dSTS

c9ac365


          Fix test

1185eaf

bgavrilMS approved these changes

View reviewed changes

gladjohn approved these changes

View reviewed changes

Contributor

gladjohn left a comment

looks good @ksaaf


          Merge branch 'main' into ksaaf/issue-5093

23a95a8

ksaaf changed the title ~~For Aad and Dsts Authority, raise error for 'organizations' tenant like 'common'~~ For Mtls, do not check for Aad and Dsts Authority type and 'organizations' or 'common' tenant

ksaaf merged commit cd5172d into main

7 checks passed

ksaaf deleted the ksaaf/issue-5093 branch

March 14, 2025 22:58

gladjohn pushed a commit that referenced this pull request


          For Mtls, do not check for Aad and Dsts Authority type and 'organizat…

8fc06c0

…ions' or 'common' tenant (#5174)

This PR removes from AcquireTokenForClientParameterBuilder.Validate checks for 'common' for Aad and Dsts Authority Types.

See conversation in this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet