bujjibabukatta opened a new pull request, #69564:
URL: https://github.com/apache/airflow/pull/69564
## Support certificate-based auth for Microsoft Graph Filesystem
Closes #69335
### What
The Microsoft Graph filesystem (`msgraph://`, `sharepoint://`,
`onedrive://`, `msgd://`
schemes, backed by `airflow.providers.microsoft.azure.fs.msgraph.get_fs`)
only supported
client-secret authentication. `KiotaRequestAdapterHook` (used by the
Microsoft Graph
operator/sensor) already supports certificate-based auth via
`certificate_path` /
`certificate_data` connection extras — this PR brings the filesystem
integration to
parity, so the same `msgraph` connection can be reused across the hook,
operator,
sensor, *and* filesystem regardless of which auth method it uses.
### Why this needed a workaround, not a one-line fix
The filesystem is built on the third-party `msgraphfs` package, whose
`AbstractMSGraphFS`
is implemented on top of `authlib`'s `AsyncOAuth2Client`. That client only
understands
OAuth2 client-secret grants — it has no concept of `azure-identity`
credential objects
like `CertificateCredential`. I traced this by walking `msgraphfs`'s and
`authlib`'s
source directly:
- `authlib`'s default `ClientAuth.DEFAULT_AUTH_METHODS` only registers
`client_secret_basic`, `client_secret_post`, and `none` — not
`private_key_jwt`, and
`msgraphfs` never calls `register_client_auth_method()` to add it. So
passing
`token_endpoint_auth_method="private_key_jwt"` through
`oauth2_client_params` would
fail at request time.
- Even if it didn't, Azure AD's certificate-based client-assertion flow
requires an
`x5t` (certificate thumbprint) JWT header that generic RFC 7523
`private_key_jwt`
signing doesn't add automatically — omitting it produces
`AADSTS700027: certificate ... not registered on application`.
- This is a known upstream limitation — the issue author already filed
`acsone/msgraphfs#13` about it.
Given that, this PR fetches a single access token up front via
`azure.identity.CertificateCredential.get_token()` (the same credential
class already
used in `KiotaRequestAdapterHook.get_credentials`) and hands it to authlib
as a static
bearer `token` in `oauth2_client_params`, instead of a `client_secret`.
**Known limitation (documented in the docs and code comment):** because
authlib isn't
driving token acquisition in this path, it can't transparently refresh the
token via the
certificate once it expires (~60–90 min, standard Azure AD access token
lifetime). Tasks
that run longer than that need to re-resolve the `ObjectStoragePath` to get
a fresh
filesystem/token. This is called out explicitly in the docs and in the
`AirflowException`
users would otherwise silently hit.
### How
- When `certificate_path` or `certificate_data` is set on the connection, it
now takes
precedence over `client_secret`.
- `certificate_password` reuses the same connection field as `client_secret`
(`conn.password`), matching `KiotaRequestAdapterHook.get_credentials`'s
existing
semantics — no new connection fields needed, and the existing `msgraph`
connection
form (which already has "Certificate path" / "Certificate data" widgets
from the hook)
works unchanged.
- Certificate auth failures are wrapped in a clear `AirflowException`
instead of
surfacing a raw `azure-identity`/`msgraphfs` traceback.
### Files changed
-
`providers/microsoft/azure/src/airflow/providers/microsoft/azure/fs/msgraph.py`
— new
`_get_certificate_token()` helper + branching logic in `get_fs()`
- `providers/microsoft/azure/tests/unit/microsoft/azure/fs/test_msgraph.py`
— 5 new
tests: cert-path token flow, cert-data + password flow, precedence over
client_secret, failure handling, existing client-secret tests unaffected
- `providers/microsoft/azure/docs/filesystems/msgraph.rst` — new
"Certificate-based
Authentication" section, including the token-refresh limitation
- `providers/microsoft/azure/newsfragments/69335.feature.rst` — new feature
note
### Testing done
- `ruff check` / `ruff format --diff` — clean
- `mypy` (isolated, no repo plugin) — clean
- Added unit tests covering: cert-path auth, cert-data + password auth,
precedence
over client_secret, credential-failure error handling, and confirmed the
existing
client-secret-only tests are unaffected
- Manually exercised `get_fs()` end-to-end (mocked
`CertificateCredential`/`MSGDriveFS`)
against 4 scenarios to confirm the exact `oauth2_client_params` built in
each case
### Not in scope / follow-ups
- Automatic token refresh for certificate auth mid-task is not implemented —
it would
require either patching `msgraphfs` upstream or subclassing `MSGDriveFS`'s
internal
client construction. Flagging for maintainers in case there's a preferred
direction
here; happy to follow up in a separate PR once `acsone/msgraphfs#13` is
resolved
upstream.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
