Merge fix-e2e-tests into PyPI-ADO-PackagePublish

Author: RyAuld

.Pipelines/template-pipeline-stages.yml

@@ -31,7 +31,7 @@ stages:
     - task: NodeTool@0
       displayName: 'Install Node.js (includes npm)'
       inputs:
-        versionSpec: 'lts/*'
+        versionSpec: '20.x'
 
     - task: securedevelopmentteam.vss-secure-development-tools.build-task-policheck.PoliCheck@2
       displayName: 'Run PoliCheck'
@@ -80,11 +80,9 @@ stages:
           python.version: '3.14'
     steps:
     # Retrieve the MSID Lab certificate from Key Vault (via AuthSdkResourceManager SC).
-    # Gated on LAB_APP_CLIENT_ID being non-empty — if e2e tests are not enabled (the default),
-    # both steps are skipped and the pipeline has no Key Vault dependency.
+    # Matches the pattern used by MSAL.js (install-keyvault-secrets.yml) and MSAL Java.
     - task: AzureKeyVault@2
       displayName: 'Retrieve lab certificate from Key Vault'
-      condition: and(succeeded(), ne(variables['LAB_APP_CLIENT_ID'], ''))
       inputs:
         azureSubscription: 'AuthSdkResourceManager'
         KeyVaultName: 'msidlabs'
@@ -98,7 +96,6 @@ stages:
         echo "##vso[task.setvariable variable=LAB_APP_CLIENT_CERT_PFX_PATH]$CERT_PATH"
         echo "Lab cert written to: $CERT_PATH ($(wc -c < "$CERT_PATH") bytes)"
       displayName: 'Write lab certificate to disk'
-      condition: and(succeeded(), ne(variables['LAB_APP_CLIENT_ID'], ''))
 
     - task: UsePythonVersion@0
       inputs:
@@ -120,13 +117,6 @@ stages:
         pytest -vv --junitxml=test-results/junit.xml 2>&1 | tee test-results/pytest.log
       displayName: 'Run tests'
       env:
-        # LAB_APP_CLIENT_ID is intentionally omitted to match the PR gate build
-        # behaviour (azure-pipelines.yml). Without it, _get_credential() in
-        # lab_config.py raises EnvironmentError and all e2e tests skip or error
-        # gracefully — identical to the PR build result.
-        # Uncomment and set this variable to enable full e2e runs on a
-        # lab-capable agent pool (requires CA-exempt network / internal agent).
-        # LAB_APP_CLIENT_ID: $(LAB_APP_CLIENT_ID)
         LAB_APP_CLIENT_CERT_PFX_PATH: $(LAB_APP_CLIENT_CERT_PFX_PATH)
 
     - task: PublishTestResults@2
@@ -141,7 +131,6 @@ stages:
     - bash: rm -f "$(Agent.TempDirectory)/lab-auth.pfx"
       displayName: 'Clean up lab certificate'
       condition: and(always(), ne(variables['LAB_APP_CLIENT_ID'], ''))
-- stage: Build
   displayName: 'Build package'
   dependsOn: CI
   condition: and(eq(dependencies.CI.result, 'Succeeded'), eq(${{ parameters.runPublish }}, true))

tests/lab_config.py

@@ -20,7 +20,6 @@
     app = get_app_config(AppSecrets.PCA_CLIENT)
 
 Environment Variables:
-    LAB_APP_CLIENT_ID: Client ID for Key Vault authentication (required)
     LAB_APP_CLIENT_CERT_PFX_PATH: Path to .pfx certificate file (required)
 """
 
@@ -43,6 +42,7 @@
     "UserConfig",
     "AppConfig",
     # Functions
+    "LAB_APP_CLIENT_ID",
     "get_secret",
     "get_user_config",
     "get_app_config",
@@ -57,6 +57,12 @@
 _MSID_LAB_VAULT = "https://msidlabs.vault.azure.net"
 _MSAL_TEAM_VAULT = "https://id4skeyvault.vault.azure.net"
 
+# Client ID for the RequestMSIDLAB app used to authenticate against the lab
+# Key Vaults. Hardcoded here following the same pattern as MSAL.NET
+# (see build/template-install-keyvault-secrets.yaml in that repo).
+# See https://docs.msidlab.com/accounts/confidentialclient.html
+LAB_APP_CLIENT_ID = "f62c5ae3-bf3a-4af5-afa8-a68b800396e9"
+
 # =============================================================================
 # Secret Name Constants
 # =============================================================================
@@ -164,6 +170,21 @@ class AppConfig:
 _msal_team_client: Optional[SecretClient] = None
 
 
+def _clean_env(name: str) -> Optional[str]:
+    """Return the env var value, or None if unset or it contains an unexpanded
+    ADO pipeline variable literal such as ``$(VAR_NAME)``.
+
+    Azure DevOps injects the literal string ``$(VAR_NAME)`` when a ``$(...)``
+    reference in a step ``env:`` block refers to a variable that has not been
+    defined at runtime.  That literal is truthy, so a plain ``os.getenv()``
+    check would incorrectly proceed as if the variable were set.
+    """
+    value = os.getenv(name)
+    if value and value.startswith("$("):
+        return None
+    return value or None
+
+
 def _get_credential():
     """
     Create an Azure credential for Key Vault access.
@@ -177,19 +198,14 @@ def _get_credential():
     Raises:
         EnvironmentError: If required environment variables are not set.
     """
-    client_id = os.getenv("LAB_APP_CLIENT_ID")
-    cert_path = os.getenv("LAB_APP_CLIENT_CERT_PFX_PATH")
+    cert_path = _clean_env("LAB_APP_CLIENT_CERT_PFX_PATH")
     tenant_id = "72f988bf-86f1-41af-91ab-2d7cd011db47"  # Microsoft tenant
-    
-    if not client_id:
-        raise EnvironmentError(
-            "LAB_APP_CLIENT_ID environment variable is required for Key Vault access")
-    
+
     if cert_path:
         logger.debug("Using certificate credential for Key Vault access")
         return CertificateCredential(
             tenant_id=tenant_id,
-            client_id=client_id,
+            client_id=LAB_APP_CLIENT_ID,
             certificate_path=cert_path,
             send_certificate_chain=True,
         )
@@ -396,7 +412,7 @@ def get_client_certificate() -> Dict[str, object]:
     Raises:
         EnvironmentError: If LAB_APP_CLIENT_CERT_PFX_PATH is not set.
     """
-    cert_path = os.getenv("LAB_APP_CLIENT_CERT_PFX_PATH")
+    cert_path = _clean_env("LAB_APP_CLIENT_CERT_PFX_PATH")
     if not cert_path:
         raise EnvironmentError(
             "LAB_APP_CLIENT_CERT_PFX_PATH environment variable is required "

tests/test_e2e.py

@@ -1,5 +1,4 @@
 """If the following ENV VAR were available, many end-to-end test cases would run.
-LAB_APP_CLIENT_ID=...
 LAB_APP_CLIENT_CERT_PFX_PATH=...
 """
 try:
@@ -29,7 +28,7 @@
 from tests.broker_util import is_pymsalruntime_installed
 from tests.lab_config import (
     get_user_config, get_app_config, get_user_password, get_secret,
-    UserSecrets, AppSecrets,
+    UserSecrets, AppSecrets, LAB_APP_CLIENT_ID,
 )
 
 
@@ -44,7 +43,23 @@
 
 _PYMSALRUNTIME_INSTALLED = is_pymsalruntime_installed()
 _AZURE_CLI = "04b07795-8ddb-461a-bbee-02f9e1bf7b46"
-_SKIP_UNATTENDED_E2E_TESTS = os.getenv("TRAVIS") or not os.getenv("CI")
+# Skip interactive / browser-dependent tests when:
+#   - on Travis CI (TRAVIS), or
+#   - on Azure DevOps (TF_BUILD) where there is no display/browser on the agent, or
+#   - not running in any CI environment at all (not CI).
+# Service-principal and ROPC tests are NOT gated on this flag; only tests that
+# call acquire_token_interactive() or acquire_token_by_device_flow() are.
+_SKIP_UNATTENDED_E2E_TESTS = (
+    os.getenv("TRAVIS") or os.getenv("TF_BUILD") or not os.getenv("CI")
+)
+
+
+def _clean_env(name):
+    """Return the env var value, or None if unset or it contains an unexpanded
+    ADO pipeline variable literal such as ``$(VAR_NAME)``."""
+    value = os.getenv(name)
+    return None if (not value or value.startswith("$(")) else value
+
 
 def _get_app_and_auth_code(
         client_id,
@@ -329,13 +344,16 @@ def test_access_token_should_be_obtained_for_a_supported_scope(self):
         self.assertIsNotNone(result.get("access_token"))
 
 
-@unittest.skipIf(os.getenv("TF_BUILD"), "Skip PublicCloud scenarios on Azure DevOps")
 class PublicCloudScenariosTestCase(E2eTestCase):
     # Historically this class was driven by tests/config.json for semi-automated runs.
-    # It now uses lab config + env vars so it can run automatically without local files.
+    # It now uses lab config + env vars so it can run automatically on any CI
+    # (including Azure DevOps) as long as LAB_APP_CLIENT_CERT_PFX_PATH is set.
 
     @classmethod
     def setUpClass(cls):
+        if not _clean_env("LAB_APP_CLIENT_CERT_PFX_PATH"):
+            raise unittest.SkipTest(
+                "LAB_APP_CLIENT_CERT_PFX_PATH not set; skipping PublicCloud e2e tests")
         pca_app = get_app_config(AppSecrets.PCA_CLIENT)
         user = get_user_config(UserSecrets.PUBLIC_CLOUD)
         cls.config = {
@@ -416,13 +434,11 @@ def test_client_secret(self):
 
     def test_subject_name_issuer_authentication(self):
         from tests.lab_config import get_client_certificate
-
-        client_id = os.getenv("LAB_APP_CLIENT_ID")
-        if not client_id:
-            self.skipTest("LAB_APP_CLIENT_ID environment variable is required")
+        if not _clean_env("LAB_APP_CLIENT_CERT_PFX_PATH"):
+            self.skipTest("LAB_APP_CLIENT_CERT_PFX_PATH not set")
 
         self.app = msal.ConfidentialClientApplication(
-            client_id,
+            LAB_APP_CLIENT_ID,
             authority="https://login.microsoftonline.com/microsoft.onmicrosoft.com",
             client_credential=get_client_certificate(),
             http_client=MinimalHttpClient())
@@ -447,35 +463,35 @@ def manual_test_device_flow(self):
 
 
 def get_lab_app(
-        env_client_id="LAB_APP_CLIENT_ID",
         env_client_cert_path="LAB_APP_CLIENT_CERT_PFX_PATH",
         authority="https://login.microsoftonline.com/"
             "72f988bf-86f1-41af-91ab-2d7cd011db47",  # Microsoft tenant ID
         timeout=None,
         **kwargs):
     """Returns the lab app as an MSAL confidential client.
 
-    Get it from environment variables if defined, otherwise fall back to use MSI.
+    Uses the hardcoded lab app client ID (RequestMSIDLAB) and a certificate
+    from the LAB_APP_CLIENT_CERT_PFX_PATH env var.
     """
     logger.info(
-        "Reading ENV variables %s and %s for lab app defined at "
+        "Reading ENV variable %s for lab app defined at "
         "https://docs.msidlab.com/accounts/confidentialclient.html",
-        env_client_id, env_client_cert_path)
-    if os.getenv(env_client_id) and os.getenv(env_client_cert_path):
+        env_client_cert_path)
+    cert_path = _clean_env(env_client_cert_path)
+    if cert_path:
         # id came from https://docs.msidlab.com/accounts/confidentialclient.html
-        client_id = os.getenv(env_client_id)
         client_credential = {
             "private_key_pfx_path":
                 # Cert came from https://ms.portal.azure.com/#@microsoft.onmicrosoft.com/asset/Microsoft_Azure_KeyVault/Certificate/https://msidlabs.vault.azure.net/certificates/LabAuth
-                os.getenv(env_client_cert_path),
+                cert_path,
             "public_certificate": True,  # Opt in for SNI
             }
     else:
         logger.info("ENV variables are not defined. Fall back to MSI.")
         # See also https://microsoft.sharepoint-df.com/teams/MSIDLABSExtended/SitePages/Programmatically-accessing-LAB-API's.aspx
         raise unittest.SkipTest("MSI-based mechanism has not been implemented yet")
     return msal.ConfidentialClientApplication(
-            client_id,
+            LAB_APP_CLIENT_ID,
             client_credential=client_credential,
             authority=authority,
             http_client=MinimalHttpClient(timeout=timeout),
@@ -831,7 +847,6 @@ def test_user_account(self):
 
 
 class WorldWideTestCase(LabBasedTestCase):
-    _ADFS_LABS_UNAVAILABLE = "ADFS labs were temporarily down since July 2025 until further notice"
 
     def test_aad_managed_user(self):  # Pure cloud
         """Test username/password flow for a managed AAD user."""
@@ -846,7 +861,6 @@ def test_aad_managed_user(self):  # Pure cloud
             scope=["https://graph.microsoft.com/.default"],
         )
 
-    @unittest.skip(_ADFS_LABS_UNAVAILABLE)
     def test_adfs2022_fed_user(self):
         """Test username/password flow for a federated user via ADFS 2022."""
         app = get_app_config(AppSecrets.PCA_CLIENT)
@@ -1162,15 +1176,13 @@ def _test_acquire_token_for_client(self, configured_region, expected_region):
         import os
         from tests.lab_config import get_client_certificate
 
-        # Get client ID from environment and certificate from lab_config
-        client_id = os.getenv("LAB_APP_CLIENT_ID")
-        if not client_id:
-            self.skipTest("LAB_APP_CLIENT_ID environment variable is required")
-        
+        # Get client ID from lab_config constant and certificate from lab_config
+        if not _clean_env("LAB_APP_CLIENT_CERT_PFX_PATH"):
+            self.skipTest("LAB_APP_CLIENT_CERT_PFX_PATH is required")
         client_credential = get_client_certificate()
 
         self.app = msal.ConfidentialClientApplication(
-            client_id,
+            LAB_APP_CLIENT_ID,
             client_credential=client_credential,
             authority="https://login.microsoftonline.com/microsoft.onmicrosoft.com",
             azure_region=configured_region,