-
Notifications
You must be signed in to change notification settings - Fork 378
[Draft] Labmigration #5558
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.
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
base: main
Are you sure you want to change the base?
[Draft] Labmigration #5558
Conversation
- Enhanced LabUserHelper with GetKVLabData and MergeKVLabData methods for direct Key Vault access - Added multi-secret merging system combining MSAL-User-Default-JSON, ID4SLAB1, and MSAL-App-Default-JSON - Updated FetchUserPassword to use KeyVaultSecretsProviderMsid for direct password retrieval - Extended FederationProvider enum with ADFSv2022 support, removed adfs2019 - Implemented comprehensive ObjectId validation against Azure AD source of truth - Fixed ByRefreshTokenTestAsync failure by correcting ObjectId mismatch in Key Vault secrets - All user and app ObjectIds now verified and aligned between Key Vault and Azure AD - Key Vault integration fully functional with 14/18 tests passing (4 failures due to Azure AD app consent/registration issues) - Complete migration from lab service API dependency to direct Azure Key Vault access
- Updated LabUserHelper with GetKVLabData and MergeKVLabData methods for complete Key Vault integration - Modified TestConstants to use new app ID and Key Vault secret names - Updated ConfidentialAppSettings to use ID4S Key Vault and new app configuration - Changed OnBehalfOfTests and LongRunningOnBehalfOfTests to use new app ID and default user method - All test configurations now point to consolidated app ID 54a2d933-8bf8-483b-a8f8-0a31924f3c1f - Key Vault integration successfully retrieves and merges secrets from multiple vaults - Assigned LabVaultAccess_UAMI to SLVM build agent for MI E2E test support
- Add comprehensive debug logging to LabUserHelper for user/app/lab retrieval tracking - Create GetDefaultAdfsUserAsync() method using MSAL-USER-FedDefault-JSON Key Vault secret - Update OnBehalfOfTests to use GetDefaultUserAsync()/GetDefaultUser2Async() instead of hardcoded emails - Standardize ADFS test method names (remove version-specific naming) - Add TLS 1.3 configuration for ADFS 2022 connectivity in integration tests - Update all ADFS tests to use new GetDefaultAdfsUserAsync() method - Improve error handling and diagnostics throughout lab infrastructure
…orrect method names and lab helper calls
…ests.NetFwk.cs with all original tests and enhanced LabUserHelper calls
… keeping main branch test structure
…lab API - resolves idlab1@msidlab4 user issue
- Updated all user references from msidlab4.onmicrosoft.com to id4slab1.onmicrosoft.com - Updated UserQueryParameters.cs with new user UPNs (MSAL-User-Default, MSAL-User-Default2, MSAL-User-XCG) - Enhanced CacheExecutionTests.cs to use LabUserHelper.GetDefaultUserAsync() instead of old lab API - Updated all unit test data in JsonHelperTests.cs, IdTokenParsingTests.cs, AADTestData.txt, UnifiedSchemaValidationTests.cs - Updated RuntimeBrokerTests.cs POP user references to [email protected] - Updated dev app references in NetWSLWam, MSIX README, and MauiApp - Migrated all federation metadata from fs.msidlab4.com to fs.id4slab1.com - Updated MSIX README to reference Azure Portal Key Vault link for password retrieval - All changes align with LabMigration Key Vault approach using enhanced LabUserHelper methods
- Fix UnifiedSchemaValidationTests to expect lowercase usernames iOS cache key generation applies ToLowerInvariant() which converts usernames to lowercase, updated test expectations accordingly - Customize IdTokenParsingTests with msal-user-default values Added new CreateAadTestTokenResponseWithMsalUserDefault() method to TestConstants.cs with custom JWT payload containing desired test values instead of real idlab token data - All previously failing tests now pass (23 passed, 1 skipped) Fixes test compatibility issues and provides custom test scenarios as requested for improved test coverage and maintenance.
Update CacheExecutionTests.cs to use new lab infrastructure API: - Changed labUser.Upn to labUser.User.Upn - Changed labUser.GetOrFetchPassword() to labUser.User.GetOrFetchPassword() - Changed labUser.AppId to labUser.App.AppId - Changed labUser.TenantId to labUser.User.TenantId This aligns with the lab migration changes where properties are now accessed through User and App objects instead of directly on LabResponse.
- Updated ConfidentialAppSettings.cs with new tenant ID (10c419d4-4a50-45b2-aa4e-919fb84df24f) and client ID (54a2d933-8bf8-483b-a8f8-0a31924f3c1f) - Updated TestConstants.cs to use id4slab1 tenant for confidential client tests - Updated ClientCredentialsTests.WithRegion.cs to use new tenant and client configurations - Updated OnBehalfOfTests.cs to use migrated tenant settings - Updated PoPTests.NetFwk.cs to use new confidential client configuration Migration results: 90/150 tests passing (60% success rate) with core authentication scenarios working. Remaining issues: Some OBO tests failing with AADSTS7000218 errors requiring KeyVault secret configuration.
- Fix OnBehalfOfTests regional test configuration to use migrated ID4SLAB1 tenant - Update LongRunningOnBehalfOfTests scope from legacy to current OBO service - Configure MSAL-APP-AzureADMultipleOrgs secret for migrated app authentication - Enable public client flows for username/password authentication scenarios - Add proper Azure AD permissions for OBO service access_as_user delegated permissions All 14 OBO integration tests now pass (100% success rate, up from 14% initially) PoP tests also fixed with proper client secret configuration
- Updated AcquireTokenFromAdfsUsernamePasswordAsync to use GetDefaultAdfsUserAsync() and new ADFS 2022 infrastructure - Updated ROPC_ADFSv4Federated_Async to use dynamic lab response instead of hardcoded constants - Migrated ConfidentialAppSettings to use new fs.id4slab1.com authority and dynamic app configuration - Updated Selenium interactive tests to use new ADFS authority with validation disabled - All ADFS tests now pass consistently with Fed1-only load balancer configuration - Fixes intermittent SSL handshake failures by using stable ADFS 2022 infrastructure
- Fixes compilation error in IdTokenParsingTests.cs - Adds method to create token response with MSAL User Default user data - Uses ID4SLAB1 tenant information matching the new infrastructure - Method follows same pattern as other CreateAadTestTokenResponse methods
- Changed ConfidentialClientID from single-tenant app (35dc5034-9b65-4a5d-ad81-73cca468c1e0) to multi-tenant app (54a2d933-8bf8-483b-a8f8-0a31924f3c1f) - Resolves AADSTS700016 'Application not found in directory' errors when using /common authority - Multi-tenant app can handle both /common and tenant-specific authorities properly - Uses same LabAuth certificate for authentication
- Updated GetTokenByAuthCode_HybridSPA_Async to use ConfidentialClientID instead of labResponse.App.AppId - Removed trailing whitespace to prevent merge conflicts - This should resolve AADSTS700016 errors in the HybridSPA test
- Updated commented tenant ID references to match the corrected tenant - Ensures consistency in both active and commented code sections
- Updated DeviceCodeFlowTestAsync and SilentTokenAfterDeviceCodeFlowWithBrokerTestAsync - Changed from default /common authority to tenant-specific authority using labResponse.User.TenantId - Resolves AADSTS50059 errors when using single-tenant apps with device code flow - Applies to both regular and broker-enabled device code scenarios
…de flow tests - Added LabUserHelper.GetDefaultUserWithMultiTenantAppAsync() using MSAL-APP-AzureADMultipleOrgs KeyVault secret - Updated DeviceCodeFlowTestAsync and SilentTokenAfterDeviceCodeFlowWithBrokerTestAsync to use new method - Removed hardcoded PublicClientID constant in favor of proper LabResponse pattern - Now uses labResponse.App.AppId which contains multi-tenant app 54a2d933-8bf8-483b-a8f8-0a31924f3c1f - Resolves AADSTS50059 errors by using multi-tenant app that supports /common authority - Follows same pattern as other LabUserHelper methods for consistency
- Add GetDefaultUserWithMultiTenantAppAsync() method to LabUserHelper - Update ConfidentialClientAuthorizationTests to use multi-tenant app - Create MSAL-APP-AzureADMultipleOrgs-JSON KeyVault secret with proper JSON structure - Replace hardcoded app IDs with multi-tenant app ID 54a2d933-8bf8-483b-a8f8-0a31924f3c1f - Resolves tenant routing issues with /common authority endpoint The multi-tenant app (AzureADMultipleOrgs) is compatible with /common authority, eliminating AADSTS700016 tenant routing conflicts that were causing test failures.
- Update InteractiveFlowTests.NetFwk.cs Interactive_AADAsync to use GetDefaultUserWithMultiTenantAppAsync - Update PoPTests.NetFwk.cs ROPC_PopTestWithRSAAsync to use GetDefaultUserWithMultiTenantAppAsync - Resolves AADSTS50194 errors for tests using /common endpoint with single-tenant app - Ensures consistent multi-tenant app usage across all affected tests
- Update InteractiveFlowTests ValidateCcsHeadersForInteractiveAuthCodeFlowAsync - Update InstanceDiscoveryIntegrationTests FailedAuthorityValidationTestAsync - Update InstanceDiscoveryIntegrationTests AuthorityValidationTestWithFalseValidateAuthorityAsync - All tests using /common endpoint now use multi-tenant app configuration - Prevents AADSTS50194 errors for single-tenant app + /common authority mismatch
- Updated OnBehalfOfTests.cs and LongRunningOnBehalfOfTests.cs to use multi-tenant app configuration - Refined ConfidentialClientAuthorizationTests.cs and InteractiveFlowTests.NetFwk.cs - Added pipeline log analysis file (60.txt) for debugging reference - All AADSTS errors resolved through systematic multi-tenant app migration - Verified test integrity preserved during lab infrastructure migration
- Changed Interactive_AADAsync to use GetDefaultUserAsync() instead of GetDefaultUserWithMultiTenantAppAsync() - Fixed InteractiveConsentPromptAsync to use default app - Fixed ValidateCcsHeadersForInteractiveAuthCodeFlowAsync to use default app - Interactive tests should use public client apps, not the multi-tenant confidential app - This resolves AADSTS7000218 errors (missing client_secret for confidential clients)
| return msalTokenResponse; | ||
| } | ||
|
|
||
| public static MsalTokenResponse CreateAadTestTokenResponseWithMsalUserDefault() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is for unit tests only I believe. Unit tests do not make HTTP calls to Entra and do not rely on an actual infra. I would recommend not modifying this.
| { | ||
| // TODO: TENANT MIGRATION - These tests currently use original tenant configuration | ||
| // Regional endpoints (eastus2.login.microsoft.com) return AADSTS100007 with new tenant | ||
| // "Only managed identities and Microsoft internal service identities are supported" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack. Let's have a list of these. We will talk to AAD to enable these features.
tests/Microsoft.Identity.Test.Integration.netcore/HeadlessTests/LongRunningOnBehalfOfTests.cs
Outdated
Show resolved
Hide resolved
| private ConfidentialClientApplication BuildCca(string tenantId, bool withRegion = false) | ||
| { | ||
| // Use migrated configuration for all tests - Updated for ID4SLAB1 tenant migration | ||
| var settings = ConfidentialAppSettings.GetSettings(Cloud.Public); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Single Responsability Principle - the ConfidentialAppSettings.GetSettings call should encapsulate this logic
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch. updated.
tests/Microsoft.Identity.Test.LabInfrastructure/LabUserHelper.cs
Outdated
Show resolved
Hide resolved
tests/Microsoft.Identity.Test.LabInfrastructure/LabUserHelper.cs
Outdated
Show resolved
Hide resolved
tests/Microsoft.Identity.Test.LabInfrastructure/LabUserHelper.cs
Outdated
Show resolved
Hide resolved
tests/Microsoft.Identity.Test.LabInfrastructure/LabUserHelper.cs
Outdated
Show resolved
Hide resolved
tests/Microsoft.Identity.Test.LabInfrastructure/LabUserHelper.cs
Outdated
Show resolved
Hide resolved
…hMultiTenantAppAsync calls - Remove redundant TestInitializeAsync calls to GetDefaultUserWithMultiTenantAppAsync() - Update test methods to call lab API only once instead of twice per test - Change TestInitialize methods from async to synchronous - Remove unused _multiTenantAppId fields - Improve RunOnBehalfOfTestAsync helper with optional parameter - Reduce test execution time and lab API load by ~50% - All 21 OBO tests verified passing
… async patterns - Change GetKVLabData return type from object to strongly-typed LabResponse - Make GetKVLabDataAsync method private (was public) - Add proper Async suffix to method name (GetKVLabData -> GetKVLabDataAsync) - Add async Key Vault operations: GetSecretByNameAsync methods to KeyVaultSecretsProvider - Make GetKVLabDataAsync properly async with await instead of Task.FromResult pattern - Update all callers to use new MergeKVLabDataAsync method - Improve error handling and remove object casting - Better performance with non-blocking Key Vault I/O operations - All builds and tests verified passing
- Remove IsValidJson helper method that was parsing JSON twice - Parse JSON directly with JsonConvert.DeserializeObject and handle JsonException - Better performance by avoiding redundant JSON parsing operations - More precise error handling with specific JsonException vs generic exceptions - Maintain same error messaging and logging behavior - All builds and tests verified passing Addresses PR feedback: 'If it's invalid JSON, throw an exception. No need to reparse.'
Overview
This PR represents a comprehensive migration to modernize the lab infrastructure used for authentication testing in MSAL.NET. The migration moves from legacy lab APIs to a new Key Vault-based approach for improved security, reliability, and maintainability.
🎯 Migration Goals
Security: Transition from legacy lab API to secure Key Vault-based credential management
Reliability: Improve test infrastructure stability and reduce external dependencies
Maintainability: Modernize codebase with enhanced debugging and error handling
Compatibility: Ensure seamless integration with existing test frameworks
🔧 Key Changes
Infrastructure Modernization
✅ Complete migration to new lab infrastructure
✅ Key Vault integration for secure credential management
✅ Enhanced LabUserHelper methods with improved error handling
✅ ADFS 2022 support and expanded authentication scenarios
Test Framework Improvements
✅ Username/Password integration tests updated with new lab methods
✅ OnBehalfOf tests migrated to Key Vault approach
✅ Hybrid SPA account handling fixes for idlab1@msidlab4 user scenarios
✅ Debug logging enhancements for better troubleshooting
Quality Assurance
✅ Fixed failing unit tests related to iOS cache key generation
✅ Custom JWT token scenarios for improved test coverage
✅ ObjectId validation in Key Vault operations
✅ Vault reference updates from buildautomation to ID4sKeyVault
🧪 Testing Strategy
The migration maintains backward compatibility while introducing modern practices:
All existing test scenarios preserved and enhanced
New Key Vault-based authentication flows validated
iOS cache key generation compatibility verified
Custom test token scenarios for reduced external dependencies
📈 Benefits
🔒 Enhanced Security: Key Vault-based credential management
⚡ Improved Performance: Reduced reliance on external lab APIs
🛠️ Better Debugging: Enhanced logging and error reporting
🔄 Future-Ready: Modern infrastructure supporting ongoing development
✅ Quality Assurance: All tests passing with improved reliability
🚀 Migration Status
Core infrastructure migration completed
Key Vault integration implemented
Test framework updates applied
Unit test compatibility issues resolved
Integration test scenarios validated
Debug and logging improvements deployed