Merge pull request #722 from microsoft/ianhelle/fix-env-auth-2023-10-02

Fix Azure env credential authentication

msticpy/_version.py

@@ -1,2 +1,2 @@
 """Version file."""
-VERSION = "2.8.0"
+VERSION = "2.9.0"

msticpy/auth/azure_auth_core.py

@@ -58,20 +58,57 @@ class AzureCredEnvNames:
     # [SuppressMessage("Microsoft.Security", "CS002:SecretInNextLine", Justification="This is an enum of env variable names")]
     AZURE_CLIENT_SECRET = "AZURE_CLIENT_SECRET"  # nosec  # noqa
 
+    # Certificate auth:
+    # A path to certificate and private key pair in PEM or PFX format
+    AZURE_CLIENT_CERTIFICATE_PATH = "AZURE_CLIENT_CERTIFICATE_PATH"
+    # (Optional) The password protecting the certificate file
+    # (for PFX (PKCS12) certificates).
+    AZURE_CLIENT_CERTIFICATE_PASSWORD = (
+        "AZURE_CLIENT_CERTIFICATE_PASSWORD"  # nosec  # noqa
+    )
+    # (Optional) Specifies whether an authentication request will include an x5c
+    # header to support subject name / issuer based authentication.
+    # When set to `true` or `1`, authentication requests include the x5c header.
+    AZURE_CLIENT_SEND_CERTIFICATE_CHAIN = "AZURE_CLIENT_SEND_CERTIFICATE_CHAIN"
+
+    # Username and password:
+    AZURE_USERNAME = "AZURE_USERNAME"  # The username/upn of an AAD user account.
+    # [SuppressMessage("Microsoft.Security", "CS002:SecretInNextLine", Justification="This is an enum of env variable names")]
+    AZURE_PASSWORD = "AZURE_PASSWORD"  # User password  # nosec  # noqa
+
+
+_VALID_ENV_VAR_COMBOS = (
+    (
+        AzureCredEnvNames.AZURE_CLIENT_ID,
+        AzureCredEnvNames.AZURE_CLIENT_SECRET,
+        AzureCredEnvNames.AZURE_TENANT_ID,
+    ),
+    (
+        AzureCredEnvNames.AZURE_CLIENT_ID,
+        AzureCredEnvNames.AZURE_TENANT_ID,
+        AzureCredEnvNames.AZURE_CLIENT_CERTIFICATE_PATH,
+    ),
+    (
+        AzureCredEnvNames.AZURE_CLIENT_ID,
+        AzureCredEnvNames.AZURE_USERNAME,
+        AzureCredEnvNames.AZURE_PASSWORD,
+        AzureCredEnvNames.AZURE_TENANT_ID,
+    ),
+)
+
 
 def _build_env_client(
     aad_uri: Optional[str] = None, **kwargs
 ) -> Optional[EnvironmentCredential]:
     """Build a credential from environment variables."""
     del kwargs
-    if (
-        AzureCredEnvNames.AZURE_CLIENT_ID not in os.environ
-        and AzureCredEnvNames.AZURE_CLIENT_SECRET not in os.environ
-    ):
-        # avoid creating env credential if require envs not set.
-        logger.info("'env' credential requested but required env vars not set")
-        return None
-    return EnvironmentCredential(authority=aad_uri)  # type: ignore
+    for env_vars in _VALID_ENV_VAR_COMBOS:
+        if all(var in os.environ for var in env_vars):
+            return EnvironmentCredential(authority=aad_uri)  # type: ignore
+
+    # avoid creating env credential if require envs not set.
+    logger.info("'env' credential requested but required env vars not set")
+    return None
 
 
 def _build_cli_client(**kwargs) -> AzureCliCredential:

msticpy/data/drivers/mordor_driver.py

@@ -442,8 +442,6 @@ def _to_datetime(date_val) -> datetime:
 
 
 # pylint: disable=not-an-iterable, no-member, too-many-instance-attributes
-
-
 @attr.s(auto_attribs=True)
 class MordorEntry:
     """Mordor data set metadata."""

msticpy/transform/proc_tree_build_winlx.py

@@ -206,7 +206,7 @@ def _extract_inferred_parents(
 ) -> pd.DataFrame:
     """Find any inferred parents and creates rows for them."""
     tz_aware = merged_procs.iloc[0][schema.time_stamp].tz
-    time_zero = pd.Timestamp(0) if tz_aware is None else pd.Timestamp(0, tz=0)
+    time_zero = pd.Timestamp(0) if tz_aware is None else pd.Timestamp(0, tz=tz_aware)
 
     # Fill in missing values for root processes
     root_procs_crit = merged_procs[Col.source_index_par].isna()

tests/auth/test_azure_auth_core.py

@@ -5,7 +5,7 @@
 # --------------------------------------------------------------------------
 """Module docstring."""
 from datetime import datetime, timedelta
-from unittest.mock import patch
+from unittest.mock import MagicMock, patch
 
 import pytest
 import pytest_check as check
@@ -14,6 +14,7 @@
 from msticpy.auth.azure_auth_core import (
     AzureCliStatus,
     AzureCloudConfig,
+    _build_env_client,
     check_cli_credentials,
     default_auth_methods,
 )
@@ -22,7 +23,7 @@
 
 __author__ = "Ian Hellen"
 
-# pylint: disable=redefined-outer-name
+# pylint: disable=redefined-outer-name, protected-access
 
 
 @pytest.fixture(scope="module")
@@ -96,12 +97,13 @@ def __init__(self, token):
 
     def get_raw_token(self):
         """Return raw token"""
-        return (tuple([*_TOKEN_WRAPPER, self.token]), None, None)
+        return (*_TOKEN_WRAPPER, self.token), None, None
 
 
 @patch(check_cli_credentials.__module__ + ".get_cli_profile")
 @pytest.mark.parametrize("test, expected", _CLI_TESTS, ids=_test_ids(_CLI_TESTS))
 def test_check_cli_credentials(get_cli_profile, test, expected):
+    # sourcery skip: use-fstring-for-concatenation
     """Test checking Azure CLI credentials."""
     test_tok = {**_TOKEN}
     test_tok.update(test[0])
@@ -111,3 +113,70 @@ def test_check_cli_credentials(get_cli_profile, test, expected):
         get_cli_profile.side_effect = test[1]
 
     check.equal(check_cli_credentials()[0], expected)
+
+
+_CLI_ID = "d8d9d2f2-5d2d-4d7e-9c5c-5d6d9d1d8d9d"
+_TENANT_ID = "f8d9d2f2-5d2d-4d7e-9c5c-5d6d9d1d8d9e"
+
+_TEST_ENV_VARS = (
+    (
+        {
+            "AZURE_CLIENT_ID": _CLI_ID,
+            "AZURE_CLIENT_SECRET": "[PLACEHOLDER]",  # nosec
+            "AZURE_TENANT_ID": _TENANT_ID,
+        },
+        True,
+    ),
+    (
+        {
+            "AZURE_CLIENT_ID": _CLI_ID,
+            "AZURE_CLIENT_CERTIFICATE_PATH": "test_cert_path",
+            "AZURE_TENANT_ID": _TENANT_ID,
+        },
+        True,
+    ),
+    (
+        {
+            "AZURE_CLIENT_ID": _CLI_ID,
+            "AZURE_USERNAME": "test_user_name",
+            "AZURE_PASSWORD": "[PLACEHOLDER]",  # nosec
+            "AZURE_TENANT_ID": _TENANT_ID,
+        },
+        True,
+    ),
+    (
+        {
+            "AZURE_CLIENT_ID": _CLI_ID,
+            "AZURE_TENANT_ID": _TENANT_ID,
+        },
+        False,
+    ),
+    (
+        {
+            "AZURE_CLIENT_CERTIFICATE_PATH": "test_cert_path",
+            "AZURE_TENANT_ID": _TENANT_ID,
+        },
+        False,
+    ),
+    ({}, False),
+)
+
+
+@pytest.mark.parametrize("env_vars, expected", _TEST_ENV_VARS)
+def test_build_env_client(env_vars, expected, monkeypatch):
+    """Test with different environment variables."""
+    # sourcery skip: no-loop-in-tests
+    for env_var, env_val in env_vars.items():
+        monkeypatch.setenv(env_var, env_val)
+
+    mock_credential = MagicMock()
+    with patch(
+        "msticpy.auth.azure_auth_core.EnvironmentCredential",
+        return_value=mock_credential,
+    ) as mock_env_cred:
+        credential = _build_env_client(aad_uri="test_aad_uri")
+
+        check.is_true((credential is not None) == expected)
+        check.is_true(
+            mock_env_cred.called_once_with(authority="test_aad_uri") or not expected
+        )