potiuk commented on PR #70879:
URL: https://github.com/apache/airflow/pull/70879#issuecomment-5150947075
Good diagnosis — the connection form does expose a `Scopes` field
(`hooks/msgraph.py:258`, defaulting to `DEFAULT_SCOPE`), but `get_fs` built
`oauth2_client_params` without a `scope` key, so `MSGDriveFS` skipped its own
default and authlib was left with nothing. A UI-configured connection could not
authenticate.
The precedence you chose is right — explicit `scope`, then the form's
`scopes`, then the Graph default — and parametrizing over all three branches is
the right way to pin it.
One thing to fix before this goes in: the two consumers of that same field
disagree on how it is delimited.
`MSGraphAsyncOperator`'s hook treats `scopes` as comma-separated and splits
it:
```python
scopes = config.get("scopes", self.scopes)
if isinstance(scopes, str):
scopes = scopes.split(",")
```
This change passes the value through verbatim into the OAuth2 `scope`
parameter, which is space-delimited by spec. So a connection configured the way
the hook expects — `"User.Read,Files.Read"` — reaches authlib as the single
literal string `"User.Read,Files.Read"`, which is one malformed scope rather
than two. Single-scope values work, which is why the current tests pass; they
only ever use one value.
That is the same shape as the bug being fixed here: one field, two
consumers, different expectations.
Something like this keeps both readings working:
```python
scopes = get_field(conn_id=conn_id, conn_type=conn_type, extras=extras,
field_name="scopes")
if isinstance(scopes, str):
scopes = " ".join(s.strip() for s in scopes.split(",") if s.strip())
oauth2_client_params["scope"] = scopes or DEFAULT_SCOPE
```
Worth a fourth parametrize case with a comma-separated value so the
behaviour is pinned rather than assumed.
Minor, while you're in there: `DEFAULT_SCOPE` is redefined here as a module
constant, but `KiotaRequestAdapterHook.DEFAULT_SCOPE` already holds the same
literal. Importing it would keep the two from drifting apart later.
---
Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting
--
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]
