Feat: Add URL-encoding detection and retry logic for cluster authentication

[vaibh123540]

This PR addresses an issue where users often provide URL-encoded passwords leading to authentication failures. The logic waits for a connection failure after which:

1. It checks for URL-encoding a password using the decodeURIComponentmethod and regex search using /%[0-9A-Fa-f]{2}/.
2. If it finds URL-encoding in the password, it suggests the user about it and offers a "retry with decoded password" option.
3. If the user chooses to retry, the connection request is made again with decoded password.
4. In case of failure user is prompted with the regular error message (no option for URL-encoding), in case of success the connection is established and the decoded password is updated in permanent storage so that the same error doesn't appear again in the next session.

Files Changed:
DocumentDBClusterItem.ts: Updated authenticateAndConnect catch block to include the described logic after a connection failure.

Testing performed:
I manually tested the workflow for an URL encoded password and a non URL encoded password. I tested both for a successful connection and an unsuccessful connection in the "retry with decoded password" logic.

The overall logic seems to have improved the User Experience.

[vaibh123540]

@microsoft-github-policy-service agree

[tnaum-ms]

Hey @vaibh123540!

Thank you so much for your contribution and for taking on this issue. Please excuse the delay in getting back to you.

You did a lot of things very well here:

• The core detection for URL-encoded passwords: the regex plus decodeURIComponent.
• You only offer the retry when the decoded password is actually different.
• Handling possible errors from decodeURIComponent and persisting the decoded password on success + masking it in context.valuesToMask

I'd suggest improvements here:

1. It would be good to only show the "retry with decoded password" option when we have a strong hint the failure is password‑related, and skip it for obvious network conditions (server offline, DNS resolution errors, etc.). You can reuse the kind of checks we already have (like the ECONNREFUSED handling in ClustersClient.ts) to distinguish those cases.

It's not easy to catch all "invalid password"-style error messages/codes across platforms (DocumentDB, Atlas, etc.), but we can have a "black list" for errors we are certain are network related. Here a generated example of what it could be:

/**
 * Network error codes where URL-encoding retry is not relevant.
 * These are OS-level error codes that indicate connectivity issues,
 * not authentication problems.
 */
const NETWORK_ERROR_PATTERNS = [
  'ECONNREFUSED', // Connection refused (server not running)
  'ENOTFOUND', // DNS lookup failed
  'ETIMEDOUT', // Connection timed out
  'ENETUNREACH', // Network unreachable
  'EHOSTUNREACH', // Host unreachable
  'EAI_AGAIN', // DNS temporary failure
  'ECONNRESET', // Connection reset by peer
  'EPIPE', // Broken pipe
  'CERT_', // Certificate errors (self-signed, expired, etc.)
  'self-signed certificate', // TLS/SSL cert issue
] as const;

function isNetworkError(errorMessage: string): boolean {
  const upperMessage = errorMessage.toUpperCase();
  return NETWORK_ERROR_PATTERNS.some((pattern) => upperMessage.includes(pattern.toUpperCase()));
}

2. The retry path is big enough now that it would be worth extracting it into a small helper with a clear set of inputs/outputs (for example, a retryConnectionWithDecodedPassword(...) function). That keeps authenticateAndConnect easier to grasp / easier to work with code maintainers in the future.
3. When you get a chance, adding some unit tests for the URL-encoding detection and retry flow would really help ensure we don't accidentally break this in the future. LLMs are particularly good at this..

Please let me know whether you've got the time to continue contributing to this PR or whether we should reassign it.

[tnaum-ms]

@vaibh123540 FYI: I merged current next into your branch. Please run npm run l10n to update the dictionary, this will fix failing checks (this isn't urgent).

[Copilot]

Pull request overview

This PR adds URL-encoding detection and retry logic for cluster authentication to improve user experience when users accidentally provide URL-encoded passwords. When a connection fails, the code detects if the password contains URL-encoded characters and offers a one-time retry with the decoded password.

Changes:

• Added URL-encoding detection using regex pattern /%[0-9A-Fa-f]{2}/ and decodeURIComponent
• Added conditional error dialog with "Retry with Decoded Password" button when URL-encoding is detected
• Implemented retry logic that updates credentials in both cache and storage with the decoded password on success
• Enhanced error messages to inform users about potential URL-encoding issues

[Copilot]

The variable name 'result' shadows the outer variable 'result' from the callWithTelemetryAndErrorHandling callback function scope. This can lead to confusion and potential bugs. Consider renaming this variable to something more specific like 'userResponse' or 'dialogResult' to avoid shadowing.

[Copilot]

The error handling uses (error as Error).message without verifying the error is actually an Error instance. While this works in most cases, it could fail if the caught error is not an Error object. For consistency with the retry error handling (line 289 which uses retryErr instanceof Error), consider using the same defensive pattern to ensure robust error message extraction.

[Copilot]

The regex pattern /%[0-9A-Fa-f]{2}/ only detects passwords that contain URL-encoded characters, but it doesn't verify that the entire password is URL-encoded or that the encoding is valid. This could result in false positives for passwords that legitimately contain sequences like "%20" as part of their actual password text (not as encoding). Consider adding additional validation to reduce false positives, or document this limitation in a comment explaining that some legitimate passwords may trigger this detection.

Suggested change

	// Heuristic: detect presence of URL-encoded-looking sequences in the password.
	// This does not guarantee that the entire password is URL-encoded or that the
	// encoding is valid; some legitimate passwords may contain sequences like "%20"
	// and will still trigger this hint. We rely on a successful decode and a changed
	// value (password !== decodedPassword) before offering the retry.

[Copilot]

The URL-encoding detection and retry logic lacks test coverage. This new feature introduces complex conditional logic, user interaction flows, and credential management that should be validated with automated tests. Consider adding tests for: (1) detection of URL-encoded passwords, (2) successful retry with decoded password, (3) failed retry with decoded password, (4) user declining the retry offer, and (5) edge cases like malformed encoding or passwords with legitimate percent sequences.

[Copilot]

The retry logic doesn't verify that authMethod is specifically NativeAuth before offering to decode the password. While the condition on line 259 checks that username and password exist (which implicitly suggests native auth), it's possible for other auth methods to have these values set. Consider adding an explicit check authMethod === AuthMethodId.NativeAuth to the shouldOfferRetry condition on line 244 to ensure this feature only applies to native authentication where passwords are actually used.

Suggested change

	password && /%[0-9A-Fa-f]{2}/.test(password) && decodedPassword && password !== decodedPassword;
	authMethod === AuthMethodId.NativeAuth &&
	password &&
	/%[0-9A-Fa-f]{2}/.test(password) &&
	decodedPassword &&
	password !== decodedPassword;

[vaibh123540]

Hey @vaibh123540!

Thank you so much for your contribution and for taking on this issue. Please excuse the delay in getting back to you.

You did a lot of things very well here:

* The core detection for URL-encoded passwords: the regex plus `decodeURIComponent`.

* You only offer the retry when the decoded password is actually different.

* Handling possible errors from `decodeURIComponent` and persisting the decoded password on success + masking it in `context.valuesToMask`

I'd suggest improvements here:

1. It would be good to only show the "retry with decoded password" option when we have a strong hint the failure is password‑related, and skip it for obvious network conditions (server offline, DNS resolution errors, etc.). You can reuse the kind of checks we already have (like the `ECONNREFUSED` handling in `ClustersClient.ts`) to distinguish those cases.

It's not easy to catch all "invalid password"-style error messages/codes across platforms (DocumentDB, Atlas, etc.), but we can have a "black list" for errors we are certain are network related. Here a generated example of what it could be:

/**
 * Network error codes where URL-encoding retry is not relevant.
 * These are OS-level error codes that indicate connectivity issues,
 * not authentication problems.
 */
const NETWORK_ERROR_PATTERNS = [
  'ECONNREFUSED', // Connection refused (server not running)
  'ENOTFOUND', // DNS lookup failed
  'ETIMEDOUT', // Connection timed out
  'ENETUNREACH', // Network unreachable
  'EHOSTUNREACH', // Host unreachable
  'EAI_AGAIN', // DNS temporary failure
  'ECONNRESET', // Connection reset by peer
  'EPIPE', // Broken pipe
  'CERT_', // Certificate errors (self-signed, expired, etc.)
  'self-signed certificate', // TLS/SSL cert issue
] as const;

function isNetworkError(errorMessage: string): boolean {
  const upperMessage = errorMessage.toUpperCase();
  return NETWORK_ERROR_PATTERNS.some((pattern) => upperMessage.includes(pattern.toUpperCase()));
}

2. The retry path is big enough now that it would be worth extracting it into a small helper with a clear set of inputs/outputs (for example, a `retryConnectionWithDecodedPassword(...)` function). That keeps `authenticateAndConnect` easier to grasp / easier to work with code maintainers in the future.

3. When you get a chance, adding some unit tests for the URL-encoding detection and retry flow would really help ensure we don't accidentally break this in the future. LLMs are particularly good at this..

Please let me know whether you've got the time to continue contributing to this PR or whether we should reassign it.

@tnaum-ms Thank you for the detailed review! I will try to make the changes you suggested by the end of this week.