Add auth_protocol support to SambaHook for Kerberos authentication #29590

[haseebmalik18]

Add auth_protocol parameter to SambaHook to support Kerberos, NTLM, and negotiate authentication. Previously the hook always used username/password auth, ignoring the {"auth": "kerberos"} connection extra, which caused SpnegoError failures for Kerberos-configured connections.

closes: #29590

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds configurable auth_protocol support to SambaHook to enable Kerberos / NTLM / negotiate auth selection and fixes prior behavior where Kerberos-configured connections were ignored.

Changes:

• Introduces auth_protocol handling in SambaHook, including validation and an optional Kerberos dependency check.
• Adds unit tests covering Kerberos (including legacy extra={"auth": "kerberos"}), NTLM, invalid protocol handling, and missing dependency behavior.
• Exposes auth_protocol in the Airflow connection UI and adds a kerberos extra dependency group for the provider.

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 4 comments.

File	Description
providers/samba/src/airflow/providers/samba/hooks/samba.py	Implements auth_protocol parsing/validation, Kerberos dependency guard, passes protocol through to smbclient, and adds UI widget.
providers/samba/tests/unit/samba/hooks/test_samba.py	Expands tests to cover new auth protocol behaviors and error cases.
providers/samba/pyproject.toml	Adds kerberos optional dependency group intended to support Kerberos auth.

[potiuk]

Sorry - we missed that there is an open question to us - > and the triage process closed it. Can you re-open and ping me again if you are still not completely put-off by that @haseebmalik18 ?

[github-actions]

uv.lock on main just moved via #68780 ("Bump zeep from 4.3.2 to 4.3.3"), commit e6c1ae6 and this PR currently conflicts.

Quickest fix:

git fetch upstream main && git rebase upstream/main
rm uv.lock && uv lock
git add uv.lock && git rebase --continue
git push --force-with-lease

Automated nudge — ignore if you're not ready to rebase. This comment is updated in place on future uv.lock bumps.

[haseebmalik18]

@potiuk ready for review, failing tests unrelated to PR

[potiuk]

The code itself looks good and is ready on the merits: auth_protocol is
validated with a ValueError, the kerberos path guards its optional deps with
AirflowOptionalProviderFeatureException, and both error paths are covered by
tests — so the "unit tests are enough" point from my earlier pass is satisfied.

The only thing blocking is CI, and it's not your change: the failing
LowestDeps job errors on AttributeError: module 'spnego' has no attribute 'ContextProxy' while collecting providers/microsoft/winrm tests — i.e. the
microsoft/winrm provider's transitive pyspnego lower bound is too low, unrelated
to adding Samba auth_protocol. Please rebase onto latest main and re-run; if it
still fails, it needs a separate bump to the winrm pyspnego floor rather than
anything here. Once CI is green this is good to approve.

---

This review was drafted by an AI-assisted tool and confirmed by an Airflow
maintainer. The findings below are observations, not blockers; an Airflow
maintainer — a real person — will take the next look at the PR. If you think a
finding is mis-applied, please reply on the PR and a maintainer will weigh in.

More on how Airflow handles maintainer review:
contributing-docs/05_pull_requests.rst.

[potiuk]

Heads-up: the failing LowestDeps job here isn't caused by this PR. It's a pre-existing gap where the microsoft/winrm provider left pyspnego unbounded, so lowest-dependency resolution picks a pre-0.4.0 pyspnego that lacks spnego.ContextProxy and pywinrm fails to import. That's being fixed in #69333. Once it merges, a rebase here should turn the shard green — nothing to change in this PR for it.

---

Drafted-by: Claude Code (Opus 4.8); reviewed by @potiuk before posting

[dabla]

You'll need to add the auth_protocol field in the provider.yaml file as well under the conn-fields section.

[dabla]

I'll try to implement a static check for this so it's automatically reported

[dabla]

So it's better to wait until the static check is merged.