Fix credential property order on HttpClientHandler

[alexeyzimarev]

Summary

• Fix property assignment order for UseDefaultCredentials and Credentials on HttpClientHandler
• In .NET Core, both properties share a backing field — setting one overwrites the other
• Now UseDefaultCredentials is set first, and Credentials is only set when non-null
• This prevents Credentials = CredentialCache.DefaultCredentials from being silently overwritten

Fixes #2236

Test plan

• Project builds successfully
• Explicit Credentials are not overwritten by UseDefaultCredentials = false
• CredentialCache.DefaultCredentials set explicitly are preserved
• UseDefaultCredentials = true works when no explicit Credentials are set
• All existing unit tests pass across all target frameworks

🤖 Generated with Claude Code

[qodo-free-for-open-source-projects]

Review Summary by Qodo

Fix credential property order on HttpClientHandler

🐞 Bug fix

Walkthroughs

Description

• Fix property assignment order for UseDefaultCredentials and Credentials on HttpClientHandler
• In .NET Core, both properties share backing field — setting one overwrites the other
• Now UseDefaultCredentials is set first, Credentials only set when non-null
• Prevents CredentialCache.DefaultCredentials from being silently overwritten

Diagram

flowchart LR
  A["HttpClientHandler<br/>property assignment"] -->|"old: Credentials first"| B["Credentials overwritten<br/>by UseDefaultCredentials"]
  A -->|"new: UseDefaultCredentials first"| C["Credentials set only<br/>if non-null"]
  C --> D["Both properties<br/>preserved correctly"]

Loading

File Changes

1. src/RestSharp/RestClient.cs 🐞 Bug fix +2/-2

Reorder HttpClientHandler property assignments

• Reordered property assignments in ConfigureHttpMessageHandler method
• Set UseDefaultCredentials before Credentials
• Added null check to only set Credentials when non-null
• Prevents property backing field conflicts in .NET Core

src/RestSharp/RestClient.cs

---

2. test/RestSharp.Tests/CredentialConfigurationTests.cs 🧪 Tests +44/-0

Add credential configuration unit tests

• Added new test class CredentialConfigurationTests with three test cases
• Test explicit credentials are not overwritten by UseDefaultCredentials
• Test CredentialCache.DefaultCredentials set explicitly are preserved
• Test UseDefaultCredentials works when no explicit credentials provided

test/RestSharp.Tests/CredentialConfigurationTests.cs

---

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (1)

1. Kerberos auth not verified 📎 Requirement gap ⛯ Reliability

Description

The added tests only assert HttpClientHandler property values and do not verify that a .NET 8
request to an integrated-auth (Kerberos/Negotiate) endpoint succeeds (non-401). This leaves the
compliance requirement for end-to-end authentication behavior on .NET 8 unverified.

Code

test/RestSharp.Tests/CredentialConfigurationTests.cs[R7-18]

+    public void Explicit_credentials_are_not_overwritten_by_UseDefaultCredentials() {
+        var credentials = new NetworkCredential("user", "password");
+        var options = new RestClientOptions("https://dummy.org") {
+            Credentials           = credentials,
+            UseDefaultCredentials = false
+        };
+
+        var handler = new HttpClientHandler();
+        RestClient.ConfigureHttpMessageHandler(handler, options);
+
+        handler.Credentials.Should().BeSameAs(credentials);
+    }

Evidence

Compliance ID 7 requires validation that .NET 8 requests using
CredentialCache.DefaultCredentials/UseDefaultCredentials successfully authenticate against a
secured endpoint (non-401). The new tests only configure HttpClientHandler and assert
handler.Credentials/handler.UseDefaultCredentials without performing any HTTP request to such an
endpoint.

Ensure DefaultCredentials/UseDefaultCredentials authenticates correctly on .NET 8 (Kerberos)
test/RestSharp.Tests/CredentialConfigurationTests.cs[7-18]
test/RestSharp.Tests/CredentialConfigurationTests.cs[34-43]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Compliance requires verifying that on Windows/.NET 8, integrated authentication (Kerberos/Negotiate via `CredentialCache.DefaultCredentials` / `UseDefaultCredentials`) actually succeeds end-to-end (non-401). Current tests only assert handler property assignment and never execute an authenticated HTTP request.

## Issue Context
The change fixes property assignment order on `HttpClientHandler`, but the compliance item specifically targets real authentication behavior on .NET 8 against a secured endpoint.

## Fix Focus Areas
- test/RestSharp.Tests/CredentialConfigurationTests.cs[1-44]
- src/RestSharp/RestClient.cs[237-238]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Document credential precedence 🐞 Bug ✓ Correctness

Description

RestClientOptions exposes both UseDefaultCredentials and Credentials, but the precedence rule
(explicit Credentials wins) is not documented, which may confuse callers who set both. Adding a
short doc note (and optionally a test) would make the behavior explicit and prevent future
regressions.

Code

src/RestSharp/RestClient.cs[R237-238]

+        handler.UseDefaultCredentials = options.UseDefaultCredentials;
+        if (options.Credentials != null) handler.Credentials = options.Credentials;

Evidence

ConfigureHttpMessageHandler sets UseDefaultCredentials and then conditionally sets
Credentials, making explicit credentials take precedence when provided. RestClientOptions
defines both properties but does not state how conflicts are resolved.

src/RestSharp/RestClient.cs[231-241]
src/RestSharp/Options/RestClientOptions.cs[77-94]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`RestClientOptions` allows setting both `Credentials` and `UseDefaultCredentials`, but the precedence rule is currently implicit (driven by assignment order in `ConfigureHttpMessageHandler`). This can confuse callers and makes the behavior easier to accidentally change in the future.

### Issue Context
`ConfigureHttpMessageHandler` sets `UseDefaultCredentials` first, then applies `Credentials` when non-null (so explicit credentials win).

### Fix Focus Areas
- src/RestSharp/Options/RestClientOptions.cs[77-94]
- src/RestSharp/RestClient.cs[231-241]
- test/RestSharp.Tests/CredentialConfigurationTests.cs[1-44]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

ⓘ The new review experience is currently in Beta. Learn more

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[cloudflare-workers-and-pages]

Deploying restsharp with    Cloudflare Pages

Latest commit:	48c27de
Status:	✅  Deploy successful!
Preview URL:	https://22f9be35.restsharp.pages.dev
Branch Preview URL:	https://fix-credential-handler-prope.restsharp.pages.dev

View logs