Skip to content

FTP: Sftp improvements [STUD-79812] [STUD-72320]#550

Open
viogroza wants to merge 1 commit into
developfrom
fix/ftp_issues
Open

FTP: Sftp improvements [STUD-79812] [STUD-72320]#550
viogroza wants to merge 1 commit into
developfrom
fix/ftp_issues

Conversation

@viogroza
Copy link
Copy Markdown
Collaborator

@viogroza viogroza commented May 14, 2026

In certain scenrios, if SshException is thrown swallow it
Add support for keyboard interactive authentification
Add unittests

https://uipath.atlassian.net/browse/STUD-72320
https://uipath.atlassian.net/browse/STUD-79812

Simple support is added for keyboard interactive authentication. The password or SecuredPassword from the card will be used for authentication. Custom prompts or 2FA Auth are not supported at the moment.

For testing purposes: the following scenario should be used: have a server that has supports only publickey and keyboard-interactive. If the password is correct, with previous version of the package it will fail to connect;with the new version it will connect.

In certain scenrios, if SshException is thrown swallow it
Add support for keyboard interactive authentification
Add unittests
@sonarqubecloud
Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
16.5% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

@viogroza
Copy link
Copy Markdown
Collaborator Author

the sonar concerns are for code that is in the unittests

Copy link
Copy Markdown

Copilot AI left a 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 improves the SFTP session in the FTP activities package by (1) adding keyboard-interactive authentication so servers configured with only publickey+keyboard-interactive can authenticate using the supplied password, and (2) tolerating bare SshException thrown by some SFTP servers from Exists() calls by treating it as "not found" rather than propagating. It also fixes a "passphare" → "passphrase" typo in resources, relaxes the no-credentials validation when UseSftp is set, and adds new unit tests for SftpSession, FileExists, and DirectoryExists activities.

Changes:

  • Always register a KeyboardInteractiveAuthenticationMethod and respond to non-echo prompts with the configured password.
  • Introduce ClientExists/SafeExists and route every _sftpClient.Exists(...) call through SafeExists to swallow bare SshException.
  • Add new test fixtures (with a TestsHelper.Flatten and CopyBool helper) and fix the "passphrase" typo in both .resx/.Designer.cs pairs; loosen WithFtpSession credential validation for SFTP.

Reviewed changes

Copilot reviewed 8 out of 10 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
Activities/FTP/UiPath.FTP/SftpSession.cs Adds keyboard-interactive auth method and the ClientExists/SafeExists wrapper used by all Exists callers.
Activities/FTP/UiPath.FTP/Properties/UiPath.FTP.resx Fixes "passphare" → "passphrase" in error message.
Activities/FTP/UiPath.FTP/Properties/UiPath.FTP.Designer.cs Regenerated comment for the corrected resource.
Activities/FTP/UiPath.FTP.Activities/WithFtpSession.cs Skips the password/cert required check when UseSftp is true.
Activities/FTP/UiPath.FTP.Activities/Properties/UiPath.FTP.Activities.resx Same "passphrase" typo fix in activities resources.
Activities/FTP/UiPath.FTP.Activities/Properties/UiPath.FTP.Activities.Designer.cs Regenerated comment for the corrected resource.
Activities/FTP/UiPath.FTP.Tests/TestsHelper.cs Adds CopyBool and Flatten helpers used by new activity tests.
Activities/FTP/UiPath.FTP.Tests/SessionsTests/SftpSessionTests.cs New tests for keyboard-interactive registration/handler and SafeExists exception handling (uses reflection on SSH.NET internals).
Activities/FTP/UiPath.FTP.Tests/ActivitiesTests/FileExistsTests.cs New activity-level tests for FileExists against a mocked IFtpSession.
Activities/FTP/UiPath.FTP.Tests/ActivitiesTests/DirectoryExistsTests.cs New activity-level tests for DirectoryExists, structurally duplicated from FileExistsTests.
Files not reviewed (2)
  • Activities/FTP/UiPath.FTP.Activities/Properties/UiPath.FTP.Activities.Designer.cs: Language not supported
  • Activities/FTP/UiPath.FTP/Properties/UiPath.FTP.Designer.cs: Language not supported
Comments suppressed due to low confidence (1)

Activities/FTP/UiPath.FTP/SftpSession.cs:71

  • Since KeyboardInteractiveAuthenticationMethod is now unconditionally added to authMethods above, authMethods.Count can never be zero, so this guard and the corresponding NoValidAuthenticationMethod exception are dead code. Either remove the guard, or keep the constructor-side validation meaningful by only adding the keyboard-interactive method when a password is actually configured (otherwise the handler will respond with string.Empty to every non-echo prompt, which is unlikely to ever succeed and is misleading at the server-side log level).
            authMethods.Add(kbiMethod);

            //Throw an error if we ended up with no authentication method
            if (authMethods.Count == 0)
            {
                throw new ArgumentNullException(Resources.NoValidAuthenticationMethod);
            }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread Activities/FTP/UiPath.FTP.Activities/WithFtpSession.cs
Comment thread Activities/FTP/UiPath.FTP/SftpSession.cs
Comment thread Activities/FTP/UiPath.FTP/SftpSession.cs
Comment thread Activities/FTP/UiPath.FTP.Tests/SessionsTests/SftpSessionTests.cs
Comment thread Activities/FTP/UiPath.FTP.Tests/ActivitiesTests/FileExistsTests.cs
Comment thread Activities/FTP/UiPath.FTP.Tests/ActivitiesTests/FileExistsTests.cs
Copy link
Copy Markdown
Contributor

@AlexMarinescuUiPath AlexMarinescuUiPath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The old code threw [NoValidAuthenticationMethod] early if you had no password and no private key. The new code skips this check entirely when [UseSftp] is true:

The problem: If someone configures an SFTP session with [UseSftp = true] but forgets to set any credential (no password, no key), the activity no longer catches this upfront. Instead it proceeds to connect, the kbi handler responds with [string.Empty], and the server rejects it with some opaque SSH authentication error.

The user gets a confusing server-side error instead of the clear message "No valid authentication method found: You need to supply either Private Key file or Password."

Better approach: Keep the validation for everyone. The kbi path already uses [ftpConfiguration.Password], so if that's set, the existing check passes anyway. You only need kbi when a password IS configured but the server doesn't accept the password auth method — in that case the validation already passes. So the [!UseSftp] escape hatch is unnecessary.

Comment thread Activities/FTP/UiPath.FTP/SftpSession.cs
Comment thread Activities/FTP/UiPath.FTP/SftpSession.cs
Comment thread Activities/FTP/UiPath.FTP/SftpSession.cs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants