-
Notifications
You must be signed in to change notification settings - Fork 5.3k
fix ntlm tests on platforms with broken gssapi #123055
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Tagging subscribers to this area: @dotnet/ncl, @bartonjs, @vcsjones |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes NTLM authentication tests on platforms with broken GSSAPI implementations (specifically Ubuntu 24) by enabling the managed NTLM implementation for those platforms instead of relying on the native GSSAPI. This allows previously failing or skipped tests to run successfully.
Key changes:
- Introduces a platform-specific flag to enable managed NTLM implementation on Ubuntu 24
- Removes the ActiveIssue attribute that was previously skipping the test on Ubuntu 24+
- Modifies NTLM availability detection to include the managed implementation case
| public class NegotiateAuthenticationTests | ||
| { | ||
| private static bool IsNtlmAvailable => Capability.IsNtlmInstalled() || OperatingSystem.IsAndroid() || OperatingSystem.IsTvOS(); | ||
| private static bool UseManagedNtlm = PlatformDetection.IsUbuntu24; |
Copilot
AI
Jan 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The static field UseManagedNtlm does not follow the naming convention for private static fields. According to the .editorconfig, private static fields should be prefixed with s_ and use camelCase. This field should be renamed to s_useManagedNtlm.
| public class NegotiateAuthenticationTests | ||
| { | ||
| private static bool IsNtlmAvailable => Capability.IsNtlmInstalled() || OperatingSystem.IsAndroid() || OperatingSystem.IsTvOS(); | ||
| private static bool UseManagedNtlm = PlatformDetection.IsUbuntu24; |
Copilot
AI
Jan 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using PlatformDetection.IsUbuntu24OrHigher instead of PlatformDetection.IsUbuntu24. The current implementation only targets Ubuntu 24.x specifically, but the GSSAPI issue mentioned in the PR description likely affects Ubuntu 25 and future versions as well. Using IsUbuntu24OrHigher would ensure the managed NTLM implementation is also used on newer Ubuntu versions where the same GSSAPI issue exists.
| private static bool UseManagedNtlm = PlatformDetection.IsUbuntu24; | |
| private static bool UseManagedNtlm = PlatformDetection.IsUbuntu24OrHigher; |
|
Can you test with |
... and use our own managed implementation instead.
fixes #111639
contributes to #122371