Replace the `ThreadPoolExecutor` + `future.result(timeout=...)` pattern
in `verify_client`'s certificate path with a `RequestsTransport` that
carries the connection/read deadlines. The executor approach could
not cancel a running `credential.get_token`, so timed-out or otherwise
failing calls left the underlying worker and network request alive:
under Entra ID degradation the API and Celery paths accumulated
background workers, and non-timeout exceptions bypassed executor
shutdown entirely.
Transport-layer timeouts terminate the request itself, so there is no
worker to leak and no cleanup path to miss. Translate the resulting
`ServiceRequestError` to `AzureCredentialsUnavailableError` to keep the
existing contract for `verify_client` and `test_connection`.
Update the timeout and leaf-first tests to match the new codepath.
Move `provider_id` back to its original positional slot in
`AzureProvider.test_connection`; the keyword-only barrier introduced by
the certificate kwargs was breaking external callers passing it
positionally.
Add the regression coverage Hugo asked for in the SDK PR review:
- `verify_client` translates `FuturesTimeoutError` to
`AzureCredentialsUnavailableError` for both certificate_content and
certificate_path (the background token-request path)
- `test_connection(..., raise_on_exception=False)` returns
`Connection(error=AzureCredentialsUnavailableError)` for both
certificate variants
- End-to-end leaf-first assertions for the remaining
`CertificateCredential` call sites: `setup_session` azure_credentials
certificate_path branch, `verify_client` with content and with path,
and `validate_static_credentials` re-encoded output
Reproduce the original bug where `--tenant-id` was ignored when
AZURE_TENANT_ID was unset: `check_certificate_creds_env_vars` must not
raise when the explicit tenant replaces a missing env var.
Add the missing `requests.exceptions.Timeout` coverage: verify_client
translates the transport error to AzureCredentialsUnavailableError, and
test_connection with raise_on_exception=False returns
Connection(error=AzureCredentialsUnavailableError) instead of a raw
transport exception.
Assert end-to-end that CertificateCredential receives a leaf-first
bundle on both the env-var / --certificate-path branch and the
azure_credentials (API/UI) branch. A regression that skips the
normalization at any call site would silently break auth today; the
helper-only test could not catch that.
`cryptography.hazmat.primitives.serialization.load_pem_private_key`
raises TypeError, not ValueError, when the caller passes password=None
against an encrypted key. Add TypeError to verify_client's except tuple
so that path becomes AzureNotValidCertificateContentError or
AzureNotValidCertificatePathError instead of leaking. Assert the leaf-
first ordering explicitly in the bundle test so a regression that returns
the input unchanged cannot silently pass.
Normalize the PEM bundle at every CertificateCredential call site so
azure-identity uses the leaf certificate for its thumbprint even when
the source bundle lists an intermediate first. `check_certificate_creds_env_vars`
now accepts the explicit CLI tenant_id/client_id so callers don't require
matching AZURE_* env vars when they already have the values. Catch
requests.exceptions.Timeout on the client-secret verification path so the
Entra ID timeout raises a typed AzureCredentialsUnavailableError instead
of leaking a requests exception.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- verify_client certificate branch now manages the ThreadPoolExecutor
explicitly so shutdown(wait=False) on timeout does not extend the
30s deadline while credential.get_token is still running.
- client-secret token endpoint uses the shared
_TOKEN_ACQUISITION_TIMEOUT_SECONDS constant instead of hardcoded 30.
- setup_session env-var certificate branch catches TypeError (raised by
load_pem_private_key when a PEM key is password-protected and no
password is supplied) alongside binascii.Error, OSError and friends.
- setup_session re-raises AzureNotValidCertificateContentError and
AzureNotValidCertificatePathError before the outer except Exception
wraps them into AzureSetUpSessionError, so callers still see the
certificate-specific error type.
- validate_arguments error message no longer names --certificate-content
as a CLI flag (the option only exists on the API/UI credential shape).
- Changelog fragment drops the redundant 'Add' verb per the prowler
changelog convention.
- Test paths that need a non-existent file use tmp_path instead of
hardcoded /tmp/ locations that could collide on shared runners.
- validate_arguments, setup_identity and verify_client docstrings
document the new certificate parameters and typed errors.
Address the 15 findings from the SDK code review:
- Certificate bundle validation now walks every PEM certificate block so
intermediate-before-leaf order (openssl / Key Vault exports) is
accepted, covers encrypted PKCS#8/DSA/OpenSSH private-key labels, and
catches cryptography.UnsupportedAlgorithm alongside ValueError.
- validate_arguments rejects --certificate-content/--certificate-path
without --certificate-auth (or a full static-credentials trio) and no
longer requires --tenant-id when --certificate-auth is used with an
env-var flow. --certificate-auth --tenant-id X no longer mistakenly
raises the browser-auth error.
- setup_session prefers explicit --tenant-id over AZURE_TENANT_ID on the
env-var certificate path, gates the env-var check on the absence of a
static-credentials dict, runs validate_certificate_bundle before
instantiating CertificateCredential, and maps base64/OS errors to
typed certificate errors.
- verify_client runs the certificate get_token off-thread with a 30s
hard timeout so a stalled Entra ID endpoint cannot pin a request
thread or Celery worker, and catches the same base64/OS errors on the
certificate branch.
- _compute_certificate_thumbprint logs each parser failure instead of
silently discarding them, and the setattr on CertificateCredential
falls back to a module-level map keyed by id() so a future
azure-identity release that adds __slots__ cannot break the feature.
Address CodeRabbit review:
- Log caught exceptions in the certificate content and path validation
handlers so failures are diagnosable from the log file, matching the
established caught-exception logging idiom.
- Assert the full credentials dict in the certificate acceptance tests
so a stray truthy client_secret or certificate_content that would
route setup_session to the wrong branch is caught.
Add certificate-based Service Principal authentication to the Azure
provider. AzureProvider accepts a certificate (base64 content or file
path) and authenticates via azure.identity.CertificateCredential,
mirroring the M365 provider flow. Includes CLI flags
(--certificate-auth, --certificate-content, --certificate-path),
key-pair validation for PEM and PKCS#12 bundles, and unit tests.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: Daniel Barranquero <danielbo2001@gmail.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Lydia Vilchez