Migrate OBO tests from old lab to ID4SLAB1#6021
Open
RyAuld wants to merge 5 commits into
Open
Conversation
- Switch OBO confidential client from client secret to LabAuth cert (msidlabs vault) - Replace AppWebApi with new AppOBOService (MSAL-APP-TodoListService) in ID4SLAB1 - Add KeyVaultSecrets.AppOBOService and AppOBOClient constants - Enable SN+I (sendX5C) for regional endpoint test - Remove commented-out old app ID references Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR migrates On-Behalf-Of (OBO) integration tests from the legacy lab tenant/app configuration to the newer ID4SLAB1 app registrations, including moving the confidential client auth for the OBO middle-tier from client secret to the LabAuth certificate.
Changes:
- Update OBO integration tests to use
KeyVaultSecrets.AppOBOServiceinstead of the legacyAppWebApiconfiguration. - Switch OBO confidential client authentication from client secret to
LabAuthcertificate retrieved from the MSIDLab Key Vault. - Add new lab Key Vault secret-name constants (
AppOBOService,AppOBOClient, and additional app constants) and update the public API baseline accordingly.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/Microsoft.Identity.Test.Integration.netcore/HeadlessTests/OnBehalfOfTests.cs | Migrates OBO test app config to ID4SLAB1 and updates confidential client auth to use LabAuth certificate (plus sendX5C for regional case). |
| tests/Microsoft.Identity.Test.Integration.netcore/HeadlessTests/LongRunningOnBehalfOfTests.cs | Migrates long-running OBO tests to ID4SLAB1 app config and switches confidential client auth to LabAuth certificate. |
| src/client/Microsoft.Identity.Lab.Api/LabInfra/KeyVaultSecrets.cs | Introduces new Key Vault secret-name constants for ID4SLAB1 OBO apps and related test apps. |
| src/client/Microsoft.Identity.Lab.Api/PublicAPI.Unshipped.txt | Updates public API baseline for newly added KeyVaultSecrets constants. |
Addresses Copilot review feedback: _keyVault (MsalTeam instance) is no longer referenced after switching to _keyVaultMsidLab for LabAuth cert. Removing to avoid CS0169 warning with TreatWarningsAsErrors. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
RyAuld
force-pushed
the
ryauld/migrate-obo-tests-to-lab1
branch
from
e782591 to
69656d5
Compare
bgavrilMS
reviewed
May 22, 2026
| /// </summary> | ||
| public const string MsalAppAzureAdMultipleOrgsRegional = "MSAL-APP-AzureADMultipleOrgsRegional-JSON"; | ||
| /// <summary> | ||
| /// Multiple orgs app in public cloud (id4slab1 tenant). Used for client credentials and regional token acquisition tests. |
Member
There was a problem hiding this comment.
Please use "multi-tenant" app term.
Contributor
Author
There was a problem hiding this comment.
Resolved - that constant (MsalAppAzureAdMultipleOrgs) is now deleted
bgavrilMS
reviewed
May 22, 2026
| /// <summary> | ||
| /// Regional app in id4slab1 tenant with SN+I claims (xms_idrel, xms_ds_cnf). Used for regional ESTS token acquisition tests. | ||
| /// </summary> | ||
| public const string MsalAppRegion = "MSAL-App-Region-JSON"; |
Member
There was a problem hiding this comment.
There are too many apps. Existing apps should be regional enabled. Pls do a replace existing apps, not an add.
Contributor
Author
There was a problem hiding this comment.
Resolved - removed MsalAppRegion and MsalAppAzureAdMultipleOrgs
bgavrilMS
reviewed
May 22, 2026
| /// <summary> | ||
| /// multiple orgs app in public cloud, with regional authority. Used for testing regional authority discovery and token acquisition in multi-tenant scenarios. | ||
| /// </summary> | ||
| public const string MsalAppAzureAdMultipleOrgsRegional = "MSAL-APP-AzureADMultipleOrgsRegional-JSON"; |
Member
There was a problem hiding this comment.
We need to have the terminlogy:
- S2S
- OBO client
- OBO service
I don't see any reason for having more than 3 apps.
Contributor
Author
There was a problem hiding this comment.
Resolved - only AppS2S, AppOBOClient, AppOBOService remain as the new constants/Apps
bgavrilMS
requested changes
May 22, 2026
Member
bgavrilMS
left a comment
bgavrilMS
left a comment
There was a problem hiding this comment.
Too many apps. But otherwise the changes look good.
Member
|
@Avery-Dunn pls review this. |
The regional test already uses AppOBOService (TodoListService) via BuildCcaAsync, making these dedicated region app constants dead code. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Migrates OBO integration tests from the old lab tenant app (23c64cd8) to the new ID4SLAB1 apps.
Changes
Infrastructure (already done)
Test Results
All 14 OBO tests pass locally (2 skipped by environment attribute).